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

[Simon Horman]

On Thu, Sep 14, 2017 at 10:35:10AM +0800, Yuanhan Liu wrote:
> 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.

Agreed.

> > and is this the most appropriate
> > place for a hardware work-around?
> 
> Maybe not. Maybe this should be workarounded inside the DPDK PMD driver.

Pushing it lower down sounds better to me.
But I'm not really in a position to judge how practical
it is to handle this in the DPDK PMD driver.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev