[ovs-dev] [PATCH v2 4/8] netdev-dpdk: implement flow put with rte flow

[Yuanhan Liu]

On Wed, Sep 13, 2017 at 11:45:30AM +0200, Simon Horman wrote:
> >     > > +        rte_memcpy(&eth_spec.dst, &match->flow.dl_dst, sizeof(eth_spec.dst));
> >     > > +        rte_memcpy(&eth_spec.src, &match->flow.dl_src, sizeof(eth_spec.src));
> >     > > +        eth_spec.type = match->flow.dl_type;
> >     > > +
> >     > > +        rte_memcpy(&eth_mask.dst, &match->wc.masks.dl_dst,
> >     > > +                   sizeof(eth_mask.dst));
> >     > > +        rte_memcpy(&eth_mask.src, &match->wc.masks.dl_src,
> >     > > +                   sizeof(eth_mask.src));
> >     > > +        eth_mask.type = match->wc.masks.dl_type;
> >     > > +
> >     > > +        add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH,
> >     > > +                         &eth_spec, &eth_mask);
> >     > > +    } else {
> >     > > +        /*
> >     > > +         * If user specifies a flow (like UDP flow) without L2 patterns,
> >     > > +         * OVS will at least set the dl_type. Normally, it's enough to
> >     > > +         * create an eth pattern just with it. Unluckily, some Intel's
> >     > > +         * NIC (such as XL710) doesn't support that. Below is a workaround,
> >     > > +         * which simply matches any L2 pkts.
> >     > > +         */
> >     > > +        add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
> >     > > +    }
> >     > 
> >     > This feels a lot like a layer violation - making the core aware
> >     > of specific implementation details of lower layers.
> >     
> >     I agree with you, but unlickily, without it, Intel NIC simply won't work
> >     (according to Finn), unless you have mac addr being provided.
> 
> I think its reasonable to provide hardware-workarounds. But is it
> appropriate to enable it for all hardware

Without the underlaying hardware info/cap given, yes, I'm afraid that
is what we can get best now.

> and is this the most appropriate
> place for a hardware work-around?

Maybe not. Maybe this should be workarounded inside the DPDK PMD driver.

	--yliu