On Thu, Mar 16, 2017 at 05:01:43PM +0000, Le Scouarnec Nicolas wrote:
>
> Hi Adrien,
>
> >On Wed, Mar 15, 2017 at 02:29:44PM +0000, Le Scouarnec Nicolas wrote:
> >> Overall, as a user, I feel one difficulty/complexity in using the API
> >> comes from the need to
> >> specify both the stack of protocol (in type) and at each level the "next
> >> protocol type" of the header (in spec).
> >>
> >> Then, the PMD has to check that what I specified as the "next protocol
> >> type" is coherent with the protocol
> >> stack before setting up the filters. Basically, for a valid filter, I
> >> should always have
> >> rte_flow_item[1].type == rte_flow_item[0].spec.type, and
> >> rte_flow_item[2].type == rte_flow_item[1].spec.{type,next_protocol}
> >> (except for L2.5 protocol as I experienced, which makes thinks
> >> confusing). Couldn't the API leverage this fact so that I don't
> >> need to specify the ether_type, TPID, next_protocol_id, ... which are
> >> redundant with rte_flow_item.type ?
>
> >Just to be clear, as a user you don't *need* to provide them, however the
> >API certainly does not prevent you to do so. Only masked fields are
> >relevant, and the default item masks (rte_flow_item_*_mask) do not include
> >protocol types because as you're pointing out, that would indeed be a pain.
>
> >Is the documentation not clear enough regarding this?
> >(see "8.2.3 Pattern item")
>
> To me it wasn't clear that the PMD/DPDK would take care of "type" fields in
> network headers for me,
> hence, I tried to set them correctly (and got it wrong for ether_type/tpid)
> -- I feared that filtering on VLAN tci
> without restricting to VLAN packets (setting ether_type) would be undefined
> behavior. I just check ixgbe_flow and
> as you said it just ignores the types and relies on the stack so my previous
> comment and suggestion
> was wrong.
Phew, I'm relieved!
> The documentation is very clear on struct and how to use them, but a few
> common examples (in C) would have been useful to me;
> for example I could have noticed that the example never set the ether_type &
> cie. testpmd is hard to read as an example.
I understand, testpmd is really meant to validate PMD functionality, it's
probably not the best implementation example to start with. I'll keep that
in mind during future evolutions.
> > I think adding custom types would be more complicated than the current
> > approach of leaving the payload type field unspecified or set it to some
> > custom value that PMDs may or may not accept depending on their
> > capabilities.
>
> You're right. My comment was based on the misconception that it was mandatory
> to correctly specify ether_types / next_protocol_id / ...
Well thanks to that you've raised an interesting issue with the VLAN item
(TBH Wenzhuo and other people warned me about that, at the time I was
certain it would not be a problem.) I'll attempt to address it as soon as
possible.
Best regards,
--
Adrien Mazarguil
6WIND