[dpdk-dev] [PATCH v3 01/25] ethdev: introduce generic flow API

[Adrien Mazarguil]

On Wed, May 24, 2017 at 03:32:02AM +0000, Zhao1, Wei wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> > Sent: Tuesday, May 23, 2017 5:51 PM
> > To: Zhao1, Wei <wei.zhao1 at intel.com>
> > Cc: dev at dpdk.org; Xing, Beilei <beilei.xing at intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v3 01/25] ethdev: introduce generic flow API
> >
> > Hi Wei,
> >
> > On Tue, May 23, 2017 at 06:07:20AM +0000, Zhao1, Wei wrote:
> > > Hi,  Adrien
> > >
> > > > +struct rte_flow_item_raw {
> > > > + uint32_t relative:1; /**< Look for pattern after the previous item. */
> > > > + uint32_t search:1; /**< Search pattern from offset (see also limit). */
> > > > + uint32_t reserved:30; /**< Reserved, must be set to zero. */
> > > > + int32_t offset; /**< Absolute or relative offset for pattern. */
> > > > + uint16_t limit; /**< Search area limit for start of pattern. */
> > > > + uint16_t length; /**< Pattern length. */
> > > > + uint8_t pattern[]; /**< Byte string to look for. */ };
> > >
> > > When I use this API to test igb flex filter, I find that in the struct
> > > rte_flow_item_raw, the member  pattern is not the same as my purpose.
> > > For example, If I type in  " flow create 0 ingress pattern raw relative is 0
> > pattern is 0123  / end actions queue index 1 / end "
> > > What I get in NIC layer is  pattern[]={ 0x30, 0x31, 0x32, 0x33, 0x0 <repeats
> > 124 times> }.
> > > But what I need is pattern[]={0x01, 0x23, 0x0 <repeats 126 times>}
> >
> > Similar limitation as I answered in [1] then. This is not a problem in the
> > rte_flow API, it's only that the testpmd parser currently provides
> > unprocessed strings to the PMD, and there is currently no method to work
> > around that.
> >
> > > About the format change of flex_filter, I have reference to the
> > > testpmd function cmd_flex_filter_parsed(), There is details of format
> > change from ASIC code to data, for example:
> > >
> > >             for (i = 0; i < len; i++) {
> > >                         c = bytes_ptr[i];
> > >                         if (isxdigit(c) == 0) {
> > >                                     /* invalid characters. */
> > >                                     printf("invalid input\n");
> > >                                     return;
> > >                         }
> > >                         val = xdigit2val(c);
> > >                         if (i % 2) {
> > >                                     byte |= val;
> > >                                     filter.bytes[j] = byte;
> > >                                     printf("bytes[%d]:%02x ", j, filter.bytes[j]);
> > >                                     j++;
> > >                                     byte = 0;
> > >                         } else
> > >                                     byte |= val << 4;
> > >             }
> > >
> > > and there is also usage example in the DPDK document testpmd_app_ug-
> > 16.11.pdf:
> > > (it also not use ASIC code)
> > >
> > > testpmd> flex_filter 0 add len 16 bytes
> > > testpmd> 0x00000000000000000000000008060000 \
> > > mask 000C priority 3 queue 3
> >
> > I understand, the difference between both commands is only that unlike
> > flex_filter, flow does not interpret the provided string as hexadecimal.
> >
> > > so, will our new generic flow API align to the old format in flex byte filter in
> > 17.08 or in the future?
> >
> > What I have in mind instead is a printf-like input method. Using the rule you
> > provided above:
> >
> >  flow create 0 ingress pattern raw relative is 0 pattern is 0123  / end actions
> > queue index 1 / end
> >
> > Will always yield "0123", however:
> >
> >  flow create 0 ingress pattern raw relative is 0 pattern is \x00\x01\x02\x03  /
> > end actions queue index 1 / end
> >
> > Will yield the intended pattern. Currently this format is interpreted as is
> > (you'll get "\x00\x01\x02\x03") however escape interpretation is in the plans.
> >
> 
> Thank you for your explanation. But there is some key point I want to repeat:
> For example, If I type in  " flow create 0 ingress pattern raw relative is 0 pattern is 0123  / end actions queue index 1 / end "
> Or maybe more accurate, " flow create 0 ingress pattern raw relative is 0 pattern is 0x0123  / end actions queue index 1 / end "
> what I need is pattern[]={0x01, 0x23, 0x0 <repeats 126 times>}.
> not  pattern[]={ 0x00, 0x01, 0x02, 0x03, 0x0 <repeats 124 times> }.
> And also, not  pattern[]={ 0x30, 0x31, 0x32, 0x33, 0x0 <repeats 124 times> }.

Right, I misread your original pattern[] intent. You would get such a
pattern by specifying \x01\x23 (just like C string literals). It would even
accept octal notation "\01\043". Both would yield { 0x01, 0x23 }.

Does something like that satisfy the requirements?

> And this problem is not a block for code develop for 17.08, but it is needed for tester and user in the feature.

Well, I've actually started implementing the above long ago in testpmd but
didn't have time to clean up the patch and submit it yet (moreover it was
not needed until now). If the idea works for your use case, I can attempt to
do that soon.

> > > At least in the struct rte_flow_item_raw, the member  pattern is the same
> > as old filter?
> >
> > It is the same as the old filter, except you cannot provide it in hexadecimal
> > format yet. No changes needed on the PMD side in any case.
> >
> > Again, this is only a testpmd implementation issue, that doesn't prevent
> > developers from creating programs that directly provide binary data to RAW
> > items, there's no such limitation.
> >
> > [1] http://dpdk.org/ml/archives/dev/2017-May/065798.html
> >
> > --
> > Adrien Mazarguil
> > 6WIND

-- 
Adrien Mazarguil
6WIND