On 16.06.2016 23:18, David Miller wrote: > From: David Ahern <d...@cumulusnetworks.com> > Date: Wed, 15 Jun 2016 14:29:28 -0600 > >> On 6/15/16 1:48 PM, David Miller wrote: >>> But if people don't use the helpers, and initialize flow structures >>> on their own, yeah that defeats the whole mechanism and things will >>> seem harder and "unhelpful". >> >> That's my point -- the flow struct does not have a consistent >> initializer. There have been a number of bug patches over the past >> year like 4cfc86f3dae6 to handle uninitialized elements. My suggestion >> here is the same as 4cfc86f3dae6 which is to initialize the flow when >> it is declared. >> >> Consider the recent change from Hannes for 38b7097b55b6. fl6 can be >> initialized when it is declared with a bit of straightforward >> refactoring -- icmp6_send has an easy split into 2. >> >> Seems like that is a more robust long term solution considering the >> various init use cases. > > The danger with initializers is it puts the burdon all over the tree. > > So now there are many places that need to be audited when a new > element is added. > > So from my perspective, the bug is that the code in question did > things by hand rather than using the helper function. > > If everyone used the flow initializer helpers, the compiler would > tell us every place that needs to be fixed up. > > By doing initializers inline, this process of discovery is more > difficult and error prone.
I see both sides and acknowledge the fact that we don't generate compile errors any more when we add fields. OTOH we already have a lot of places, probably even more, which don't use the initialization helper functions as they just need to feed in a subset of the flowi4 information (memset and fl4.daddr = <>; and some other attributes is a common case. Most of the time a new attribute that gets added to flow4i only decreases the scope of the actual lookup, thus a missing attribute doesn't necessarily indicate a bug. In my local branch I adapted the changes from Julian so far. I will revisit the initialization of flowi4, thanks for all the input so far. Currently I have difficulties keeping user space behavior the same while improving the ECN situation, as ToS simply specifies the use of the second bit. I can't improve the ECN situation in a transparent way. Bye, Hannes
