Hi, On Fri, Mar 10, 2017 at 07:12:24AM +0000, Lu, Wenzhuo wrote: > Some replies in line. > Send it again with off the us...@dpdk.org. Seems I cannot send the mail > successfully with it.
I'm removing everyone from the CC list and putting back dev@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. [...] > 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. > 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. 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
