[patch libteam] initscripts: need add the port before update config

[Hangbin Liu]

After c8b356a3cd36 ("teamd: config: update local prio to kernel"),
we update team port prio not only on local, but also on kernel side.

So we need to add the port to team first before update TEAM_PORT_CONFIG.
Or the teamd_config_port_update() would be failed because we could not find
the port in teamd_get_port_by_ifname() during teamd_config_port_set().

Reported-by: LiLiang <liali(a)redhat.com&gt;
Fixes: c8b356a3cd36 ("teamd: config: update local prio to kernel")
Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com&gt;
---
 teamd/redhat/initscripts/network-scripts/ifup-TeamPort | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
index 8c3cc3a..954b47a 100755
--- a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
+++ b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
@@ -51,8 +51,8 @@ if [ -n "${TEAM_MASTER}" ]; then
 		exit 0
 	fi
 	/sbin/ip link set dev ${DEVICE} down
+	/usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1
 	if [ -n "${TEAM_PORT_CONFIG}" ]; then
 		/usr/bin/teamdctl ${TEAM_MASTER} port config update ${DEVICE}
"${TEAM_PORT_CONFIG}" || exit 1
 	fi
-	/usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1
 fi
-- 
2.19.2

[Xin Long]

On Sat, Jun 15, 2019 at 5:41 AM Hangbin Liu <liuhangbin(a)gmail.com&gt; wrote:
>  After c8b356a3cd36 ("teamd: config: update local prio to kernel"),
>  we update team port prio not only on local, but also on kernel side.
>
>  So we need to add the port to team first before update TEAM_PORT_CONFIG.
>  Or the teamd_config_port_update() would be failed because we could not find
>  the port in teamd_get_port_by_ifname() during teamd_config_port_set().
>
>  Reported-by: LiLiang <liali(a)redhat.com&gt;
>  Fixes: c8b356a3cd36 ("teamd: config: update local prio to kernel")
>  Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com&gt;
>  ---
>   teamd/redhat/initscripts/network-scripts/ifup-TeamPort | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
>  diff --git a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
> b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
>  index 8c3cc3a..954b47a 100755
>  --- a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
>  +++ b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
>  @@ -51,8 +51,8 @@ if [ -n "${TEAM_MASTER}" ]; then
>                  exit 0
>          fi
>          /sbin/ip link set dev ${DEVICE} down
>  +       /usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1
>          if [ -n "${TEAM_PORT_CONFIG}" ]; then
>                  /usr/bin/teamdctl ${TEAM_MASTER} port config update ${DEVICE}
> "${TEAM_PORT_CONFIG}" || exit 1
>          fi
>  -       /usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1
>   fi 
Ohh, it seems that this fixes the same issue as the patch I just posted,

  [PATCH] teamd: return 0 if tdport doesn't exist in teamd_config_port_set

This issue actually also breaks NM-team, which means if you insist
on your patch, you've got to ask NM to do the same, it will kinda
drop the "feature" (port conf can go first). More importantly, it
will break old use.

> --
> 2.19.2
> _______________________________________________
> libteam mailing list -- libteam(a)lists.fedorahosted.org
> To unsubscribe send an email to libteam-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedorahosted.org/archives/list/[e-mail]

[Jiri Pirko]

Mon, Jul 01, 2019 at 09:57:18AM CEST, lucien.xin(a)gmail.com wrote:
> On Sat, Jun 15, 2019 at 5:41 AM Hangbin Liu
> <liuhangbin(a)gmail.com&gt; wrote:
> >
> > After c8b356a3cd36 ("teamd: config: update local prio to kernel"),
> > we update team port prio not only on local, but also on kernel side.
> >
> > So we need to add the port to team first before update TEAM_PORT_CONFIG.
> > Or the teamd_config_port_update() would be failed because we could not find
> > the port in teamd_get_port_by_ifname() during teamd_config_port_set().
> >
> > Reported-by: LiLiang <liali(a)redhat.com&gt;
> > Fixes: c8b356a3cd36 ("teamd: config: update local prio to kernel")
> > Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com&gt;
> > ---
> >  teamd/redhat/initscripts/network-scripts/ifup-TeamPort | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
> b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
> > index 8c3cc3a..954b47a 100755
> > --- a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
> > +++ b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort
> > @@ -51,8 +51,8 @@ if [ -n "${TEAM_MASTER}" ]; then
> >                 exit 0
> >         fi
> >         /sbin/ip link set dev ${DEVICE} down
> > +       /usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1
> >         if [ -n "${TEAM_PORT_CONFIG}" ]; then
> >                 /usr/bin/teamdctl ${TEAM_MASTER} port config update ${DEVICE}
> "${TEAM_PORT_CONFIG}" || exit 1
> >         fi
> > -       /usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1
> >  fi
> Ohh, it seems that this fixes the same issue as the patch I just posted,
>
>   [PATCH] teamd: return 0 if tdport doesn't exist in teamd_config_port_set
>
> This issue actually also breaks NM-team, which means if you insist
> on your patch, you've got to ask NM to do the same, it will kinda
> drop the "feature" (port conf can go first). More importantly, it
> will break old use. 
I'll rather like to fix this in teamd, as Xin is suggesting. Dropping
this patch.


>
>> --
>> 2.19.2
>> _______________________________________________
>> libteam mailing list -- libteam(a)lists.fedorahosted.org
>> To unsubscribe send an email to libteam-leave(a)lists.fedorahosted.org
>> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>> List Archives:
https://lists.fedorahosted.org/archives/list/[e-mail]