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

[Mordechay Haimovsky <motih@mellanox.com>]

The mlx4 does 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.

Moti

> -----Original Message-----
> From: Shahaf Shuler
> Sent: Thursday, January 25, 2018 5:38 PM
> To: Mordechay Haimovsky <motih@mellanox.com>; Adrien Mazarguil
> <adrienm@mellanox.com>
> Cc: dev@dpdk.org; Mordechay Haimovsky <motih@mellanox.com>;
> stable@dpdk.org
> Subject: RE: [dpdk-stable] [PATCH] net/mlx4: fix no broadcasts reception in
> promisc
> 
> Sunday, January 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).
> > + * 2. All multicast:
> > + *       port MAC/VLAN + broadcast + catch-all multicast.
> > + * 3. Otherwise:
> > + *       port MAC/VLAN + broadcast MAC/VLAN.
> >   *
> >   * About MAC flow rules:
> >   *
> > @@ -1304,9 +1307,6 @@ struct mlx4_drop {
> >  		!priv->dev->data->promiscuous ?
> >  		&vlan_spec.tci :
> >  		NULL;
> > -	int broadcast =
> > -		!priv->dev->data->promiscuous &&
> > -		!priv->dev->data->all_multicast;
> >  	uint16_t vlan = 0;
> >  	struct rte_flow *flow;
> >  	unsigned int i;
> > @@ -1340,7 +1340,7 @@ struct mlx4_drop {
> >  			rule_vlan = NULL;
> >  		}
> >  	}
> > -	for (i = 0; i != RTE_DIM(priv->mac) + broadcast; ++i) {
> > +	for (i = 0; i != RTE_DIM(priv->mac) + 1; ++i) {
> >  		const struct ether_addr *mac;
> >
> >  		/* Broadcasts are handled by an extra iteration. */ @@ -
> > 1404,7 +1404,7 @@ struct mlx4_drop {
> >  			goto next_vlan;
> >  	}
> >  	/* Take care of promiscuous and all multicast flow rules. */
> > -	if (!broadcast) {
> > +	if (priv->dev->data->promiscuous || priv->dev->data->all_multicast)
> 
> Please help me to understand the issue.
> The promisc and allmulti spec and mask contains the broadcast addresses.
> They are not received because of the ibv flow rule type (MC_DEFAULT /
> ALL_DEFAULT) ?
> 
> > {
> >  		for (flow = LIST_FIRST(&priv->flows);
> >  		     flow && flow->internal;
> >  		     flow = LIST_NEXT(flow, next)) {
> > --
> > 1.8.3.1