Aaron Conole <[email protected]> writes:

> Eelco Chaudron <[email protected]> writes:
>
>> On 4 Apr 2022, at 23:54, Paolo Valerio wrote:
>>
>>> Aaron Conole <[email protected]> writes:
>>>
>>>> Paolo Valerio <[email protected]> writes:
>>>>
>>>>> The next header contained in the last extension header of the IPv6
>>>>> later frags still contain the information of the upper layer protocol
>>>>> number.
>>>>>
>>>>> Similarly to what OvS does for IPv4, allow L4 matches for later IPv6
>>>>> frags as well by processing later frags and storing the nw_proto
>>>>> information.
>>>>>
>>>>> Signed-off-by: Paolo Valerio <[email protected]>
>>>>> ---
>>>>
>>>> Digging in, I could never find a reason why ipv6 was treated differently
>>>> in this manner, but I guess there could be cases where the header layout
>>>> could cause problems (see the top of the while() block).  I don't know
>>>> that it's a practical concern (other values are probably less common),
>>>> but it was the only thing I could come up with for why the match might
>>>> behave this way.  I haven't done enough mailing list archeology to have
>>>> a good idea.
>>>
>>> Guess this may probably be due to (rfc8200 section-4.5):
>>>
>>> "The subsequent fragment packets are composed of:
>>>
>>>    [...]
>>>
>>>    (2)  A Fragment header containing:
>>>
>>>            The Next Header value that identifies the first header
>>>            after the Per-Fragment headers of the original packet.
>>>
>>>            [...]"
>>>
>>> where the "first header" in the original packet could be another
>>> extension header or an upper-layer header, while the Fragment header is
>>> the last EH for later fragments (according to section 4.5).
>
> Thanks for digging in and finding this.
>
>>> If I'm not missing something, it makes sense to drop the patch.
>>> It's probably a good thing if we document the different behavior between
>>> IPv4 and IPv6 for later frags, though.
>>
>> Alternatively, we could check if the next header is a non-extension header 
>> and if so, mark it as such.
>>
>> The only problem might be the case where packets do have extension
>> headers after the fragmentation header, and they might be dropped
>> (handled differently).
>
> I think it might be best to keep the system as-is.  If we add this
> "feature," users will still need to do matches like:
>
>   table=X,ip6,ct_state=-trk actions=ct(...)
>
> They can try to optimize the flow table looks by doing individual
> selections, but it will be confusing to flow writers (and take an extra
> bit of time to debug) when they get missed packets.  Even if we try to
> make things convenient, there can be edge cases we miss - and today
> systems like OVN already accommodate the nuance of v4 vs v6.
>

I agree. Although the proposal is reasonable and covers the majority of
the cases, it's probably better to stick with the current behavior.

> I'm in favor of a documentation patch instead that spells it out for
> flow writers to clearly understand.  That way we don't have a custom
> state machine to try and work around this limitation.
>

If we all agree, I'd proceed with that.

>> Anyhow, whatever we decide, we should document it in both the documentation 
>> and code.
>>
>> //Eelco

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to