Tue, Oct 03, 2017 at 08:35:54PM CEST, t...@quantonium.net wrote: >On Tue, Oct 3, 2017 at 12:46 AM, Jiri Pirko <j...@resnulli.us> wrote: >> Fri, Sep 29, 2017 at 07:59:35PM CEST, t...@herbertland.com wrote: >>>On Fri, Sep 29, 2017 at 10:42 AM, David Miller <da...@davemloft.net> wrote: >>>> From: Tom Herbert <t...@herbertland.com> >>>> Date: Fri, 29 Sep 2017 08:48:55 -0700 >>>> >>>>> 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?
It may be that currently it affects only flower. That does not mean you should add flower-specific quirk. All I say is this should be handled in a generic way, independent on the caller. > >> 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? Sure, what I ment was any change of behaviour. > >> 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. I don't think that the overhead would be much different from what you proposed. > >Tom > > >>
