Re: [PATCH] Fix remove and add a member port from one team to another.

[Samudrala, Sridhar]

On 11/19/2015 12:37 AM, Jiri Pirko wrote:
>  Wed, Nov 18, 2015 at 09:29:07PM CET, sridhar.samudrala(a)intel.com
> wrote:
> > The following scenario where a member port is removed from team1
> > and added to team2
> >
> >    teamd -d -c '{"device": "team1", "ports":
> {"p4p1": {}}}'
> >    teamd -d -t team2
> >    ip link set p4p1 nomaster
> >    ip link set p4p1 master team2
> >
> > fails with this error
> >
> > teamd_team2[17226]: p4p1: Port original hardware address has different length (0)
> than team device has (6).
> >
> > This patch fixes this issue by initializing orig_hw_addr/len of a port
> > after it is removed from another team so that the correct address is
> > available when it is added to the new team.
> >
> > It doesn't fix the case where the member port is directly moved to
> > a new team without removing it from the original team.
> >
> >    teamd -d -c '{"device": "team1", "ports":
> {"p4p1": {}}}'
> >    teamd -d -t team2
> >    ip link set p4p1 master team2
> >
> > In this scenario teamd gets a notification only after the port is added
> > to the new team.
> > It is not clear how to address this scenario OR if we should block
> > adding a member port to a team if it is already a member of another team.
> >
> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala(a)intel.com&gt;
> > ---
> > libteam/ifinfo.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c
> > index 47b342c..632f5f1 100644
> > --- a/libteam/ifinfo.c
> > +++ b/libteam/ifinfo.c
> > @@ -116,6 +116,11 @@ static void update_hwaddr(struct team_ifinfo *ifinfo, struct
> rtnl_link *link)
> > 			memcpy(ifinfo->orig_hwaddr, hwaddr, hwaddr_len);
> > 		set_changed(ifinfo, CHANGED_HWADDR);
> > 	}
> > +
> > +	if (!ifinfo->master_ifindex && !ifinfo->orig_hwaddr_len) {
> > +		ifinfo->orig_hwaddr_len = hwaddr_len;
> > +		memcpy(ifinfo->orig_hwaddr, hwaddr, hwaddr_len);
> > +	}
>  This does not look correct to me.
>
>  How the addr len becomes 0? 
It doesn't become 0. It is not initialized in this
scenario as the port 
is a member of another team and master_ifindex points to that team.
 From update_hwaddr()
         if (ifinfo->hwaddr_len != hwaddr_len) {
                 ifinfo->hwaddr_len = hwaddr_len;
                 if (!ifinfo->master_ifindex)
                         ifinfo->orig_hwaddr_len = hwaddr_len;
                 set_changed(ifinfo, CHANGED_HWADDR_LEN);
         }


> > }
> >
> > static void update_ifname(struct team_ifinfo *ifinfo, struct rtnl_link *link)
> > -- 
> > 2.1.0
> >