[ovs-dev] [PATCH] lib/flow.c: Don't include ports of first fragments in hash

[Darrell Ball]

There is a switch config, called 'frag_mode'; the default is 'normal' where
tcp ports, udp ports
and ICMP type/code are set to 0 for all fragments.

http://www.openvswitch.org/support/dist-docs/ovs-ofctl.8.pdf

This is referenced during datapath flow translation right now.
It might be worth investigating whether this config can be referenced and
folded in earlier.




On Wed, Jun 5, 2019 at 3:36 PM Van Bemmel, Jeroen (Nokia - US) <
jeroen.van_bemmel at nokia.com> wrote:

> For a series of IP fragments, only the first packet includes the transport
> header (TCP/UDP/SCTP) and the src/dst ports. By including these port
> numbers in the hash, it may happen that a first fragment hashes to a
> different value than subsequent packets, causing different packets from
> the same flow to follow different paths. This in turn may result in
> out-of-order delivery or failed reassembly. This patch excludes port
> numbers from the hash calculation in case of IP fragmentation.
>
> Signed-off-by: Jeroen van Bemmel <jeroen.van_bemmel at nokia.com>
> ---
> diff --git a/lib/flow.c b/lib/flow.c
> index f39b57f5b..0ff20a87a 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -2370,8 +2370,10 @@ flow_hash_symmetric_l3l4(const struct flow *flow,
> uint32_t basis,
>      }
>      hash = hash_add(hash, flow->nw_proto);
> -    if (flow->nw_proto == IPPROTO_TCP || flow->nw_proto == IPPROTO_SCTP ||
> -         (inc_udp_ports && flow->nw_proto == IPPROTO_UDP)) {
> +    if ((flow->nw_proto == IPPROTO_TCP || flow->nw_proto == IPPROTO_SCTP
> ||
> +        (inc_udp_ports && flow->nw_proto == IPPROTO_UDP))
> +        /* For first fragments, don't include ports in hash */
> +        && !(flow->nw_frag & FLOW_NW_FRAG_MASK) ) {
>          hash = hash_add(hash,
>                          (OVS_FORCE uint16_t) (flow->tp_src ^
> flow->tp_dst));
>      }
> @@ -2486,7 +2488,8 @@ flow_mask_hash_fields(const struct flow *flow,
> struct flow_wildcards *wc,
>          break;
>      case NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP:
> -        if (is_ip_any(flow) && flow->nw_proto == IPPROTO_UDP) {
> +        if (is_ip_any(flow) && flow->nw_proto == IPPROTO_UDP
> +            && !(flow->nw_frag & FLOW_NW_FRAG_MASK) ) {
>              memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
>              memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
>          }
> @@ -2503,7 +2506,8 @@ flow_mask_hash_fields(const struct flow *flow,
> struct flow_wildcards *wc,
>          }
>          memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> -        if (flow->nw_proto == IPPROTO_TCP || flow->nw_proto ==
> IPPROTO_SCTP) {
> +        if ((flow->nw_proto == IPPROTO_TCP || flow->nw_proto ==
> IPPROTO_SCTP)
> +             && OVS_LIKELY(!(flow->nw_frag & FLOW_NW_FRAG_MASK))) {
>              memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src);
>              memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
>          }
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>