Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx4: fix no broadcasts reception in promisc

[Shahaf Shuler <shahafs@mellanox.com>]

Thursday, January 25, 2018 7:12 PM, Mordechay Haimovsky:
not support "don't care" bits in its flow spec mask, therefore
> we do cannot use The following rules used by mlx5 for promiscuous and
> allmulti:
>         if (dev->data->promiscuous) {
>                 struct rte_flow_item_eth promisc = {
>                         .dst.addr_bytes = "\x00\x00\x00\x00\x00\x00",
>                         .src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
>                         .type = 0,
>                 };
> 
>                 claim_zero(mlx5_ctrl_flow(dev, &promisc, &promisc));
>                 return 0;
>         }
>         if (dev->data->all_multicast) {
>                 struct rte_flow_item_eth multicast = {
>                         .dst.addr_bytes = "\x01\x00\x00\x00\x00\x00",
>                         .src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
>                         .type = 0,
>                 };
> 
> What we can use instead  In order to place the device in promiscuous mode
> is:
>     struct all_flow_attr {
>         struct ibv_flow_attr     attr;
>     } __attribute__((packed)) all_attr = {
>         .attr = {
>             .comp_mask  = 0,
>             .type       = IBV_FLOW_ATTR_ALL_DEFAULT,
>             .size       = sizeof(all_attr),
>             .priority   = 0,
>             .num_of_specs = 0,
>             .port       = PORT_NUM,
>             .flags      = 0,
>         },
>     };
>     struct ibv_flow *all_flow;
>     all_flow = ibv_create_flow(qp, &all_attr.attr);
> 
> Now *_DEFAULT rules are low priority rules which catch the appropriate
> traffic which was not caught by another rule.
> There is probably another rule for BC which is added in the driver  that
> catches the BC, that is why we do not see it.
> When we add an additional BC rule, we will also get it.

Thanks, understood. 

7, 2018 1:05 PM, Moti Haimovsky
> > > Subject: [dpdk-stable] [PATCH] net/mlx4: fix no broadcasts reception
> > > in promisc
> > >
> > > This patch fixes the issue of mlx4 not receiving broadcast packets
> > > when configured to work promiscuous mode.
> > >
> > > Fixes: eacaac7bae36 ("net/mlx4: restore promisc and allmulti
> > > support")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> > > ---
> > >  drivers/net/mlx4/mlx4_flow.c | 16 ++++++++--------
> > >  1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx4/mlx4_flow.c
> > > b/drivers/net/mlx4/mlx4_flow.c index 69025da..ec13f5a 100644
> > > --- a/drivers/net/mlx4/mlx4_flow.c
> > > +++ b/drivers/net/mlx4/mlx4_flow.c
> > > @@ -1223,9 +1223,12 @@ struct mlx4_drop {
> > >   *
> > >   * Various flow rules are created depending on the mode the device is in:
> > >   *
> > > - * 1. Promiscuous: port MAC + catch-all (VLAN filtering is ignored).
> > > - * 2. All multicast: port MAC/VLAN + catch-all multicast.
> > > - * 3. Otherwise: port MAC/VLAN + broadcast MAC/VLAN.
> > > + * 1. Promiscuous:
> > > + *       port MAC + broadcast + catch-all (VLAN filtering is ignored).

No need to add here anything. Promiscuous includes broadcasts by definition.  

> > > + * 2. All multicast:
> > > + *       port MAC/VLAN + broadcast + catch-all multicast.

No need to add here anything. All multi includes broadcasts by definition.  

If no further comments from Adrien, and you agree, I can fix those two during the merge (no need for another version).

Adrien - please have a look on it tomorrow 

Reviewed-by: Shahaf Shuler <shahafs@mellanox.com>