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

[Chas Williams <3chas3@gmail.com>]

On Wed, Aug 1, 2018 at 9:04 AM Radu Nicolau <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>
> ---
>  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.


> -       case BONDING_MODE_8023AD:
> -               break;
>         /* Promiscuous mode is propagated only to primary slave */
>         case BONDING_MODE_ACTIVE_BACKUP:
>         case BONDING_MODE_TLB:
> @@ -2645,12 +2643,10 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev
> *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_disable(internals->slaves[i].port_id);
>                 break;
> -       /* In mode4 promiscus mode is set managed when slave is
> added/removed */
> -       case BONDING_MODE_8023AD:
> -               break;
>         /* Promiscuous mode is propagated only to primary slave */
>         case BONDING_MODE_ACTIVE_BACKUP:
>         case BONDING_MODE_TLB:
> --
> 2.7.5
>
>