On 21 Jun 2022, at 22:48, Rosemarie O'Riorden wrote:
> When a packet enters the OVS datapath and does not match any existing
> flows installed in the kernel flow cache, the packet will be sent to
> userspace to be parsed, and a new flow will be created. The kernel and
> OVS rely on each other to parse packet fields in the same way so that
> packets will be handled properly.
>
> As per the design document linked below, OVS expects all later IPv6
> fragments to have nw_proto=44 in the flow key, so they can be correctly
> matched on OpenFlow rules. OpenFlow controllers create pipelines based
> on this design.
>
> This behavior was changed by the commit in the Fixes tag so that
> nw_proto equals the next_header field of the last extension header.
> However, there is no counterpart for this change in OVS userspace,
> meaning that this field is parsed differently between OVS and the
> kernel. This is a problem because OVS creates actions based on what is
> parsed in userspace, but the kernel-provided flow key is used as a match
> criteria, as described in Documentation/networking/openvswitch.rst. This
> leads to issues such as packets incorrectly matching on a flow and thus
> the wrong list of actions being applied to the packet. Such changes in
> packet parsing cannot be implemented without breaking the userspace.
>
> The offending commit is partially reverted to restore the expected
> behavior.
>
> The change technically made sense and there is a good reason that it was
> implemented, but it does not comply with the original design of OVS.
> If in the future someone wants to implement such a change, then it must
> be user-configurable and disabled by default to preserve backwards
> compatibility with existing OVS versions.
>
> Cc: [email protected]
> Fixes: fa642f08839b ("openvswitch: Derive IP protocol number for IPv6 later
> frags")
> Link: https://docs.openvswitch.org/en/latest/topics/design/#fragments
> Signed-off-by: Rosemarie O'Riorden <[email protected]>
> ---
> net/openvswitch/flow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 372bf54a0ca9..e20d1a973417 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -407,7 +407,7 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct
> sw_flow_key *key)
> if (flags & IP6_FH_F_FRAG) {
> if (frag_off) {
> key->ip.frag = OVS_FRAG_TYPE_LATER;
> - key->ip.proto = nexthdr;
> + key->ip.proto = NEXTHDR_FRAGMENT;
> return 0;
> }
> key->ip.frag = OVS_FRAG_TYPE_FIRST;
> --
> 2.35.3
Thanks Rosemarie, for fixing this. The change looks good to me!
Acked-by: Eelco Chaudron <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
