[dpdk-dev] FW: Issues with ixgbe and rte_flow

[Lu, Wenzhuo]

Hi Adrien,

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Friday, March 10, 2017 7:46 PM
> To: Lu, Wenzhuo
> Cc: Le Scouarnec Nicolas; dev at dpdk.org
> Subject: Re: FW: Issues with ixgbe and rte_flow
> 
> Hi,
> 
> On Fri, Mar 10, 2017 at 07:12:24AM +0000, Lu, Wenzhuo wrote:
> > Some replies in line.
> > Send it again with off the users at dpdk.org. Seems I cannot send the mail
> successfully with it.
> 
> I'm removing everyone from the CC list and putting back dev at dpdk.org then,
> let's not break everyone's DPDK-related spam filters anymore.
> 
> This is separate from the VLAN item issue mentioned in the same thread, I think
> this one is related to the ixgbe implementation (sorry Wenzhuo :) More below.
No need to sorry :) It's my bad. Forgot to mention that this rte_flow_error problem is related to the PMD implementation. 
More or less it caused by the HW. Although we have this generic filter, but in HW there're still several different filters, PMD has to check them one by one.

> 
> [...]
> > Hi Le Scouarnec,
> >
> > > -----Original Message-----
> > > From: Le Scouarnec Nicolas
> [...]
> > > I also have a side comment which might be more related to the
> > > general rte_flow API than to the specific implementation in ixgbe. I
> > > don't know if it is specific to ixgbe's implementation but, as a
> > > user, the rte_flow_error returned was not very useful for it does
> > > return the error of the last tried filter-type (L2 tunnel in ixgbe),
> > > and not the error of the filter-type that my setup should use (flow director).
> 
> The helpfulness of error messages is entirely a PMD's responsibility since they
> are not hard-coded into the API. rte_flow is deliberately not aware of the
> various underlying APIs used by PMDs to implement flow rules.
> 
> In this case, assuming your rule could only work through flow director, the PMD
> should have saved and reported the error encountered with that filter type
> (either by saving it before attempting others, or recognizing that this rule
> wouldn't work with others and not attempt them).
> 
> > > I had to change the order in which filters are tried in ixgbe code
> > > to get a more useful error message. As NICs can have several
> > > filter-types, It would be be useful to the user if
> > > rte_flow_validate/create could return the errors for all filter
> > > types tried although that would require to change the rte_flow API
> > > and returning an array of rte_flow_error and not a single struct.
> 
> rte_flow_error is a compromise to provide a detailed explanation about the
> errno value returned by a function, which describes exactly one error (ideally the
> first error encountered).
> 
> While returning an array could provide additional details about subsequent
> errors, I think it would needlessly complicate the API and make it slower without
> much benefit, given that most (if not all) PMD functions return as soon as one
> error is detected and also for performance reasons.
> 
> > It's a good suggestion.  I remember we have some discussion about how
> > to feedback the error to the APP. I think the reason why we don't make
> > it too complex because it's the first step of generic API.  Now we see
> > some feedback from the users, we can keep optimizing it :)
> 
> Right. Note ixgbe could append several messages to rte_flow_error.message if
> necessary as in such cases. Storage for the message is provided by the PMD and
> can be const, static or dynamic.
> 
> However I really think the best approach would be to report the most relevant
> (first) error only.
It's good if we can find which one is the most relevant. It sounds like introducing an AI to judge which kind of the filter is the best choice.
And considering some filters may have overlap, like n-tuple and flow director.  So maybe the first one is the only option here :)

> 
> > And about the tpid, ethertype. I have a thought that why we need it as it's
> duplicate with the item type. I think the initial design is just following the IEEE
> spec to define the structures so we will not miss anything. But why not do some
> optimization. For VLAN the tpid must be 0x8100, for IPv4, the ethertype must be
> 0x0800. So why bothering let APP provide them and driver check them? Seems
> we can just remove these fields from the structures, it can make things simpler.
> >
> > Adrien, as you're the maintainer of rte_flow, any thought about these ideas?
> Thanks.
> 
> Basically I think we must give users the flexibility to provide nonstandard TPIDs
> as well (there's apparently already a few), so we can't just leave it out entirely.
Agree that TPID or something else like that can be changed. But my point is when using the filter, users don't care about the value of TPID, they only care about the vlan packets should be filtered. The type already tells the driver that. No matter we use the well-known or self-defined TPID, it's duplicate of vlan type.

> 
> It's really about whether we want to make the inner type part of the VLAN item
> with TPID outside or keep it as-is. Anyway please reply to my previous message
> if you want to talk about that and let's fork this one to discuss the
> rte_flow_error issue.
> 
> --
> Adrien Mazarguil
> 6WIND