On Tue, Oct 3, 2017 at 12:46 AM, Jiri Pirko [off-list ref] wrote:

Fri, Sep 29, 2017 at 07:59:35PM CEST, tom@herbertland.com wrote:

quoted

On Fri, Sep 29, 2017 at 10:42 AM, David Miller [off-list ref] wrote:

quoted

From: Tom Herbert <redacted>
Date: Fri, 29 Sep 2017 08:48:55 -0700

quoted

The flow_dissector interface is not a uAPI.

That's not true, insofar as cls_flower.c uses the flow_dissector
therefore if you change the flow_dissector in certain ways then
cls_flower.c might have it's behavior changed and that is in fact UAPI
facing.

Then I would suggest adding another flag like FLOW_DISSECTOR_F_FLOWER
and when anyone puts new code into flow_dissector they can wrap it
with "if !(flags & FLOW_DISSECTOR_F_FLOWER)". If the flower uAPI is
subsequently update then the conditional can be removed. This way
flower can support maintain its APIs, but we can still still extend
and improve flow_dissector for othersuse cases.

This is not flower-specific problem. Flow_dissector is a servant of many.

Besides flower, what other use cases of flow_dissector have made
flow_dissector interface a uAPI? Any use of hashing does not do this.
Maybe OVS does?

As such, it is instructed what should it do. If you want to
change the way inner headers are parsed, you should either:

Why would that only affect the way inner headers are parsed? Wouldn't
we need to consider any change to flow_dissector that might affect the
output in any way. For instance, the depth limits I added would change
to output for someone that was parsing thirty-five layers of
encapsulation so it it looks like that feature needs a flag. What if
someone adds a new Ethernet protocol or a new encap protocol?

1) change the callers so they are behaving the same as before
2) make the flow_dissection change optional so the caller can say if he
   wants original or new behaviour.

I guess we can do that, but am concerned about the overhead this will
generate if were adding a flag each time anyone modifies the function.
There are performance critical use cases of flow_dissector that will
be impacted by such changes.

Tom