[PATCH iproute2] ip link: fix up bond/bridge slave id

[PATCH iproute2] ip link: fix up bond/bridge slave id

[Hangbin Liu]

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 ip/iplink_bond_slave.c   | 2 +-
 ip/iplink_bridge_slave.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
index 9c60dea..8a3104b 100644
--- a/ip/iplink_bond_slave.c
+++ b/ip/iplink_bond_slave.c
@@ -130,7 +130,7 @@ static void bond_slave_print_help(struct link_util *lu, int argc, char **argv,
 }
 
 struct link_util bond_slave_link_util = {
-	.id		= "bond",
+	.id		= "bond_slave",
 	.maxattr	= IFLA_BOND_SLAVE_MAX,
 	.print_opt	= bond_slave_print_opt,
 	.parse_opt	= bond_slave_parse_opt,
diff --git a/ip/iplink_bridge_slave.c b/ip/iplink_bridge_slave.c
index a44d4e4..58563e0 100644
--- a/ip/iplink_bridge_slave.c
+++ b/ip/iplink_bridge_slave.c
@@ -293,7 +293,7 @@ static void bridge_slave_print_help(struct link_util *lu, int argc, char **argv,
 }
 
 struct link_util bridge_slave_link_util = {
-	.id		= "bridge",
+	.id		= "bridge_slave",
 	.maxattr	= IFLA_BRPORT_MAX,
 	.print_opt	= bridge_slave_print_opt,
 	.parse_opt	= bridge_slave_parse_opt,
-- 
2.5.5

Re: [PATCH iproute2] ip link: fix up bond/bridge slave id

[Nikolay Aleksandrov]

> On Sep 6, 2016, at 10:44 AM, Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> ip/iplink_bond_slave.c   | 2 +-
> ip/iplink_bridge_slave.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
> index 9c60dea..8a3104b 100644
> --- a/ip/iplink_bond_slave.c
> +++ b/ip/iplink_bond_slave.c
> @@ -130,7 +130,7 @@ static void bond_slave_print_help(struct link_util *lu, int argc, char **argv,
> }
> 
> struct link_util bond_slave_link_util = {
> -	.id		= "bond",
> +	.id		= "bond_slave",
> 	.maxattr	= IFLA_BOND_SLAVE_MAX,
> 	.print_opt	= bond_slave_print_opt,
> 	.parse_opt	= bond_slave_parse_opt,
> diff --git a/ip/iplink_bridge_slave.c b/ip/iplink_bridge_slave.c
> index a44d4e4..58563e0 100644
> --- a/ip/iplink_bridge_slave.c
> +++ b/ip/iplink_bridge_slave.c
> @@ -293,7 +293,7 @@ static void bridge_slave_print_help(struct link_util *lu, int argc, char **argv,
> }
> 
> struct link_util bridge_slave_link_util = {
> -	.id		= "bridge",
> +	.id		= "bridge_slave",
> 	.maxattr	= IFLA_BRPORT_MAX,
> 	.print_opt	= bridge_slave_print_opt,
> 	.parse_opt	= bridge_slave_parse_opt,
> -- 
> 2.5.5
> 

NAK, this will actually force the iplink slave logic to always lookup the linkutil symbol. Please check iplink.c and how slave linkutil structs are found.
The first part of the “bridge/bond_slave” is extracted from the requested type (e.g. bond_slave -> bond) and is
looked up with the slave flag set to true, in case that fails the symbol (%s_slave_link_util) is looked up and added to the
linkutil list for later use, but in this case we can end up with linkutil list head pointing to itself (the same slave link util added twice if f.e. we have
more than 1 slave of that type) and since it doesn’t match - we’ll loop forever.

Just try adding 2 or more bond slaves with this patch and then do:
ip -d link show

Re: [PATCH iproute2] ip link: fix up bond/bridge slave id

[Hangbin Liu]

On Tue, Sep 06, 2016 at 11:31:24AM +0300, Nikolay Aleksandrov wrote:
> NAK, this will actually force the iplink slave logic to always lookup the linkutil symbol. Please check iplink.c and how slave linkutil structs are found.
> The first part of the “bridge/bond_slave” is extracted from the requested type (e.g. bond_slave -> bond) and is
> looked up with the slave flag set to true, in case that fails the symbol (%s_slave_link_util) is looked up and added to the
> linkutil list for later use, but in this case we can end up with linkutil list head pointing to itself (the same slave link util added twice if f.e. we have
> more than 1 slave of that type) and since it doesn’t match - we’ll loop forever.
> 
> Just try adding 2 or more bond slaves with this patch and then do:
> ip -d link show
> 

Ohh, my bad. Thanks for the detailed explanation. I will go through the code
and check it again.

Thanks
Hangbin