DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index
@ 2017-11-20  7:15 Lilijun (Jerry)
  2017-11-23 16:53 ` Kyle Larose
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lilijun (Jerry) @ 2017-11-20  7:15 UTC (permalink / raw)
  To: dev, tomaszx.kulasek; +Cc: Zhang, Jerry, Wanghanlin

Hi all,

In the function bond_mode_8023ad_enable(), the var i is to used to as the second parameter to pass the slave dev's dpdk port id to the function bond_mode_8023ad_activate_slave(). 
I think this variable is only a index for array internals->active_slaves. So its need to be fixed and change i to internals->active_slaves[i].


[Patch] net/bonding: fix bond 8023ad mode enable using wrong index.
Signed-off-by: Lilijun <jerry.lilijun@huawei.com>

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index a2313b3..20a08dc 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1159,7 +1159,7 @@
        uint8_t i;

        for (i = 0; i < internals->active_slave_count; i++)
-               bond_mode_8023ad_activate_slave(bond_dev, i);
+               bond_mode_8023ad_activate_slave(bond_dev, internals->active_slaves[i]);

        return 0;
}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index
  2017-11-20  7:15 [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index Lilijun (Jerry)
@ 2017-11-23 16:53 ` Kyle Larose
  2017-11-23 22:56   ` Kyle Larose
  2017-11-23 22:57 ` Kyle Larose
  2017-12-12 15:58 ` Doherty, Declan
  2 siblings, 1 reply; 6+ messages in thread
From: Kyle Larose @ 2017-11-23 16:53 UTC (permalink / raw)
  To: Lilijun (Jerry), dev, tomaszx.kulasek; +Cc: Zhang, Jerry, Wanghanlin

Hello,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lilijun (Jerry)
> Sent: Monday, November 20, 2017 2:16 AM
> To: dev@dpdk.org; tomaszx.kulasek@intel.com
> Cc: Zhang, Jerry; Wanghanlin
> Subject: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable
> using wrong index
> 

...

> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1159,7 +1159,7 @@
>         uint8_t i;
> 
>         for (i = 0; i < internals->active_slave_count; i++)
> -               bond_mode_8023ad_activate_slave(bond_dev, i);
> +               bond_mode_8023ad_activate_slave(bond_dev, internals-
> >active_slaves[i]);
> 
>         return 0;
> }
> 


I don't think active_slaves is correct here. The slave is not yet active, so it may not be in that array yet.
Should we instead use internals->slaves[i].port_id?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index
  2017-11-23 16:53 ` Kyle Larose
@ 2017-11-23 22:56   ` Kyle Larose
  0 siblings, 0 replies; 6+ messages in thread
From: Kyle Larose @ 2017-11-23 22:56 UTC (permalink / raw)
  To: Kyle Larose
  Cc: Lilijun (Jerry), dev, tomaszx.kulasek, Zhang, Jerry, Wanghanlin

Sorry -- ignore my previous email. I was looking at it from the wrong
perspective. I was thinking of what happens in active activate_slave, which
calls bond_mode_8023ad_activate_slave *before* adding to the
active_slaves[] array.

In this case, the slaves are already active, since we're iterating over the
active slave list.

On Thu, Nov 23, 2017 at 11:53 AM, Kyle Larose <klarose@sandvine.com> wrote:

> Hello,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lilijun (Jerry)
> > Sent: Monday, November 20, 2017 2:16 AM
> > To: dev@dpdk.org; tomaszx.kulasek@intel.com
> > Cc: Zhang, Jerry; Wanghanlin
> > Subject: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable
> > using wrong index
> >
>
> ...
>
> > --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> > +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> > @@ -1159,7 +1159,7 @@
> >         uint8_t i;
> >
> >         for (i = 0; i < internals->active_slave_count; i++)
> > -               bond_mode_8023ad_activate_slave(bond_dev, i);
> > +               bond_mode_8023ad_activate_slave(bond_dev, internals-
> > >active_slaves[i]);
> >
> >         return 0;
> > }
> >
>
>
> I don't think active_slaves is correct here. The slave is not yet active,
> so it may not be in that array yet.
> Should we instead use internals->slaves[i].port_id?
>
>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index
  2017-11-20  7:15 [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index Lilijun (Jerry)
  2017-11-23 16:53 ` Kyle Larose
@ 2017-11-23 22:57 ` Kyle Larose
  2017-12-12 15:58 ` Doherty, Declan
  2 siblings, 0 replies; 6+ messages in thread
From: Kyle Larose @ 2017-11-23 22:57 UTC (permalink / raw)
  To: Lilijun (Jerry), dev, tomaszx.kulasek; +Cc: Zhang, Jerry, Wanghanlin

Hello,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lilijun (Jerry)
> Sent: Monday, November 20, 2017 2:16 AM
> To: dev@dpdk.org; tomaszx.kulasek@intel.com
> Cc: Zhang, Jerry; Wanghanlin
> Subject: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable
> using wrong index
> 
> Hi all,
> 
> In the function bond_mode_8023ad_enable(), the var i is to used to
> as the second parameter to pass the slave dev's dpdk port id to the
> function bond_mode_8023ad_activate_slave().
> I think this variable is only a index for array internals-
> >active_slaves. So its need to be fixed and change i to internals-
> >active_slaves[i].
> 
> 
> [Patch] net/bonding: fix bond 8023ad mode enable using wrong index.
> Signed-off-by: Lilijun <jerry.lilijun@huawei.com>
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
> b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index a2313b3..20a08dc 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1159,7 +1159,7 @@
>         uint8_t i;
> 
>         for (i = 0; i < internals->active_slave_count; i++)
> -               bond_mode_8023ad_activate_slave(bond_dev, i);
> +               bond_mode_8023ad_activate_slave(bond_dev, internals-
> >active_slaves[i]);
> 
>         return 0;
> }
> 

I have tested this, and it alleviates some of the issues I was reporting earlier with link aggregator selection.

Tested-by: Kyle Larose <klarose@sandvine.com>

Thanks,

Kyle

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index
  2017-11-20  7:15 [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index Lilijun (Jerry)
  2017-11-23 16:53 ` Kyle Larose
  2017-11-23 22:57 ` Kyle Larose
@ 2017-12-12 15:58 ` Doherty, Declan
  2017-12-15 20:48   ` Ferruh Yigit
  2 siblings, 1 reply; 6+ messages in thread
From: Doherty, Declan @ 2017-12-12 15:58 UTC (permalink / raw)
  To: Lilijun (Jerry), dev, tomaszx.kulasek; +Cc: Zhang, Jerry, Wanghanlin

On 20/11/2017 7:15 AM, Lilijun (Jerry) wrote:
> Hi all,
> 
> In the function bond_mode_8023ad_enable(), the var i is to used to as the second parameter to pass the slave dev's dpdk port id to the function bond_mode_8023ad_activate_slave().
> I think this variable is only a index for array internals->active_slaves. So its need to be fixed and change i to internals->active_slaves[i].
> 
> 
> [Patch] net/bonding: fix bond 8023ad mode enable using wrong index.
> Signed-off-by: Lilijun <jerry.lilijun@huawei.com>
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index a2313b3..20a08dc 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1159,7 +1159,7 @@
>          uint8_t i;
> 
>          for (i = 0; i < internals->active_slave_count; i++)
> -               bond_mode_8023ad_activate_slave(bond_dev, i);
> +               bond_mode_8023ad_activate_slave(bond_dev, internals->active_slaves[i]);
> 
>          return 0;
> }
> 
> 

Acked-by: Declan Doherty <declan.doherty@intel.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index
  2017-12-12 15:58 ` Doherty, Declan
@ 2017-12-15 20:48   ` Ferruh Yigit
  0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2017-12-15 20:48 UTC (permalink / raw)
  To: Doherty, Declan, Lilijun (Jerry), dev, tomaszx.kulasek
  Cc: Zhang, Jerry, Wanghanlin

On 12/12/2017 7:58 AM, Doherty, Declan wrote:
> On 20/11/2017 7:15 AM, Lilijun (Jerry) wrote:
>> Hi all,
>>
>> In the function bond_mode_8023ad_enable(), the var i is to used to as the second parameter to pass the slave dev's dpdk port id to the function bond_mode_8023ad_activate_slave().
>> I think this variable is only a index for array internals->active_slaves. So its need to be fixed and change i to internals->active_slaves[i].

    net/bonding: fix activated slave in 8023ad mode

    In the function bond_mode_8023ad_enable(), the var i is used as slave
    port id to the function bond_mode_8023ad_activate_slave().

    This variable is only a index for array internals->active_slaves.
    So its need to be fixed and change i to internals->active_slaves[i].

    Fixes: 46fb43683679 ("bond: add mode 4")
    Cc: stable@dpdk.org

>>
>>
>> [Patch] net/bonding: fix bond 8023ad mode enable using wrong index.
>> Signed-off-by: Lilijun <jerry.lilijun@huawei.com>

Tested-by: Kyle Larose <klarose@sandvine.com>

> Acked-by: Declan Doherty <declan.doherty@intel.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-15 20:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-20  7:15 [dpdk-dev] [PATCH] net/bonding: fix bond 8023ad mode enable using wrong index Lilijun (Jerry)
2017-11-23 16:53 ` Kyle Larose
2017-11-23 22:56   ` Kyle Larose
2017-11-23 22:57 ` Kyle Larose
2017-12-12 15:58 ` Doherty, Declan
2017-12-15 20:48   ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).