Re: [dpdk-dev] [PATCH] net/bonding: propagate promiscous mode in mode 4

[Radu Nicolau <radu.nicolau@intel.com>]

On 8/1/2018 2:34 PM, Chas Williams wrote:
>
>
> On Wed, Aug 1, 2018 at 9:04 AM Radu Nicolau <radu.nicolau@intel.com 
> <mailto:radu.nicolau@intel.com>> wrote:
>
>     Update the bonding promiscuous mode enable/disable functions as to
>     propagate the change to all slaves instead of doing nothing; this
>     seems to be the correct behaviour according to the standard,
>     and also implemented in the linux network stack.
>
>     Signed-off-by: Radu Nicolau <radu.nicolau@intel.com
>     <mailto:radu.nicolau@intel.com>>
>     ---
>      drivers/net/bonding/rte_eth_bond_pmd.c | 8 ++------
>      1 file changed, 2 insertions(+), 6 deletions(-)
>
>     diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
>     b/drivers/net/bonding/rte_eth_bond_pmd.c
>     index ad6e33f..16105cb 100644
>     --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>     +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>     @@ -2617,12 +2617,10 @@ bond_ethdev_promiscuous_enable(struct
>     rte_eth_dev *eth_dev)
>             case BONDING_MODE_ROUND_ROBIN:
>             case BONDING_MODE_BALANCE:
>             case BONDING_MODE_BROADCAST:
>     +       case BONDING_MODE_8023AD:
>                     for (i = 0; i < internals->slave_count; i++)
>     rte_eth_promiscuous_enable(internals->slaves[i].port_id);
>                     break;
>     -       /* In mode4 promiscus mode is managed when slave is
>     added/removed */
>
>
> This comment is true (and it appears it is always on in 802.3ad mode):
>
>         /* use this port as agregator */
>         port->aggregator_port_id = slave_id;
>         rte_eth_promiscuous_enable(slave_id);
>
> If we are going to do this here, we should probably get rid of it in
> the other location so that future readers aren't confused about which
> is the one doing the work.
>
> Since some adapters don't have group multicast support, we might
> already be in promiscuous anyway.  Turning off promiscuous for
> the bonding master might turn it off in the slaves where an application
> has already enabled it.

The idea was to preserve the current behavior except for the explicit 
promiscuous disable/enable APIs; an application may disable the 
promiscuous mode on the bonding port and then enable it back, expecting 
it to propagate to the slaves.