Re: [PATCH RFC net-next 0/2] net:sched: Introduce tc flower2 classifier based on PANDA parser in kernel

On Wed, Sep 22, 2021 at 3:32 PM Marcelo Ricardo Leitner
[off-list ref] wrote:

On Wed, Sep 22, 2021 at 10:28:41AM -0700, Tom Herbert wrote:

quoted

On Wed, Sep 22, 2021 at 8:49 AM Simon Horman [off-list ref] wrote:

quoted

On Wed, Sep 22, 2021 at 07:42:58AM -0700, Tom Herbert wrote:

quoted

On Tue, Sep 21, 2021 at 9:46 PM Jiri Pirko [off-list ref] wrote:

quoted

Wed, Sep 22, 2021 at 06:38:20AM CEST, xiyou.wangcong@gmail.com wrote:

quoted

On Thu, Sep 16, 2021 at 1:02 PM Felipe Magno de Almeida
[off-list ref] wrote:

quoted

The PANDA parser, introduced in [1], addresses most of these problems
and introduces a developer friendly highly maintainable approach to
adding extensions to the parser. This RFC patch takes a known consumer
of flow dissector - tc flower - and  shows how it could make use of
the PANDA Parser by mostly cutnpaste of the flower code. The new
classifier is called "flower2". The control semantics of flower are
maintained but the flow dissector parser is replaced with a PANDA
Parser. The iproute2 patch is sent separately - but you'll notice
other than replacing the user space tc commands with "flower2"  the
syntax is exactly the same. To illustrate the flexibility of PANDA we
show a simple use case of the issues described in [2] when flower
consumes PANDA. The PANDA Parser is part of the PANDA programming
model for network datapaths, this is described in
https://github.com/panda-net/panda.

My only concern is that is there any way to reuse flower code instead
of duplicating most of them? Especially when you specifically mentioned
flower2 has the same user-space syntax as flower, this makes code
reusing more reasonable.

Exactly. I believe it is wrong to introduce new classifier which would
basically behave exacly the same as flower, only has different parser
implementation under the hood.

Could you please explore the possibility to replace flow_dissector by
your dissector optionally at first (kernel config for example)? And I'm
not talking only about flower, but about the rest of the flow_dissector
users too.

+1

quoted

Hi Jiri,

Yes, the intent is to replace flow dissector with a parser that is
more extensible, more manageable and can be accelerated in hardware
(good luck trying to HW accelerate flow dissector as is ;-) ). I did a
presentation on this topic at the last Netdev conf:
https://www.youtube.com/watch?v=zVnmVDSEoXc. FIrst introducing this
with a kernel config is a good idea.

Can we drop hyperbole? There are several examples of hardware that
offload (a subset of) flower. That the current kernel implementation has
the properties you describe is pretty much irrelevant for current hw
offload use-cases.

Simon,

"current hw offload use-cases" is the problem; these models offer no
extensibility. For instance, if a new protocol appears or a user wants
to support their own custom protocol in things like tc-flower there is
no feasible way to do this. Unfortunately, as of today it seems, we
are still bound by the marketing department at hardware vendors that
pick and choose the protocols that they think their customers want and
are willing to invest in-- we need to get past this once and for all!

Not that I don't agree with this, but I'm having a hard time seeing
how flower2 would be more flexible than current approach in this
sense. Say that someone wants to add support for IPv64. AFAICS it
would still require changes to iproute, cls_flower2, panda and
drivers, which is the complain that I usually hear about cls_flower
extensibility.

Yes, flower2 is not sufficient and neither would be replacing the flow
dissector with PANDA. In order to make this tc-flower truly extensible
we need "generic tc-flower" which I touched upon in the Netdev. This
really means that we need to coordinate three actors of TC
application, kernel, and hardware for offloads. Adding a new protocol
means that we have to be able to parse the packet before match rules
can be created, so that means the parser is a first class citizen in
making this work.

TCP options too, for example. The 1st patch has code like:

+#define PANDA_METADATA_tcp_options                                     \
+       struct {                                                        \
+               __u16 mss;                                              \
+               __u8 window_scaling;                                    \
+               struct {                                                \
+                       __u32 value;                                    \
+                       __u32 echo;                                     \
+               } timestamp;                                            \
+               struct {                                                \
+                       __u32 left_edge;                                \
+                       __u32 right_edge;                               \
+               } sack[PANDA_TCP_MAX_SACKS];                                    \
+       } tcp_options

...

+#define PANDA_METADATA_TEMP_tcp_option_mss(NAME, STRUCT)               \
+static void NAME(const void *vopt, void *iframe,                       \
+                struct panda_ctrl_data ctrl)                           \
+{                                                                      \
+       const struct tcp_opt_union *opt = vopt;                         \
+       struct STRUCT *frame = iframe;                                  \
+                                                                       \
+       frame->tcp_options.mss = ntohs(opt->mss);                       \
+}

So if we have a new option on the game, what will need updating?

It's a matter of adding new options to the parser program and then
downloading the parser either to kernel or hardware for offload (again
they really should be running identical programs). Given that we need
to be able to set up rules for matching fields in the protocols which
can be done by using offsets as the keys in an abstract way. The
kernel and the user space application then need to be tightly
coordinated such that they have agreement that they are talking about
the same fields for a given value (BTF might help here). The last part
of the equation is to dynamically create a TC CLI for the new fields,
this could be derived from the input parser program that is annotated
with human readable names for protocols and fields.

Thanks,
Tom

  Marcelo

quoted

IMO, what we need is a common way to extend the kernel, tc, and other
applications for new protocols and features, but also be able to apply
that method to extend to the hardware which is _offloading_ kernel
functionality which in this case is flow dissector. The technology is
there to do this as programmable NICs for instance are the rage, but
we do need to create common APIs to be able to do that. Note this
isn't just tc, but a whole space of features; for instance, XDP hints
is nice idea for the NIC to provide information about protocols in a
packet, but unless/until there is a way to program the device to pull
out arbitrary information that the user cares about like something
from their custom protocol, then it's very limited utility...

Tom