Thanks, Greg. I pushed the patches you sent for all the appropriate branches
(2.5-2.8). I sent out patches to do bug patch releases for all those branches,
too, that will include these. Hopefully we'll get those merged and all the
releases out tomorrow.
--Justin
> On Sep 4, 2019, at 10:07 AM, Greg Rose <gvrose8...@gmail.com> wrote:
>
> Upstream commit:
> commit ad06a566e118e57b852cab5933dbbbaebb141de3
> Author: Greg Rose <gvrose8...@gmail.com>
> Date: Tue Aug 27 07:58:09 2019 -0700
>
> openvswitch: Properly set L4 keys on "later" IP fragments
>
> When IP fragments are reassembled before being sent to conntrack, the
> key from the last fragment is used. Unless there are reordering
> issues, the last fragment received will not contain the L4 ports, so the
> key for the reassembled datagram won't contain them. This patch updates
> the key once we have a reassembled datagram.
>
> The handle_fragments() function works on L3 headers so we pull the L3/L4
> flow key update code from key_extract into a new function
> 'key_extract_l3l4'. Then we add a another new function
> ovs_flow_key_update_l3l4() and export it so that it is accessible by
> handle_fragments() for conntrack packet reassembly.
>
> Co-authored-by: Justin Pettit <jpet...@ovn.org>
> Signed-off-by: Greg Rose <gvrose8...@gmail.com>
> Acked-by: Pravin B Shelar <pshe...@ovn.org>
> Signed-off-by: David S. Miller <da...@davemloft.net>
>
> Signed-off-by: Greg Rose <gvrose8...@gmail.com>
>
> ---
>
> V2 - Fix compile errors
> ---
> datapath/conntrack.c | 5 ++
> datapath/flow.c | 157 +++++++++++++++++++++++++++++----------------------
> datapath/flow.h | 1 +
> 3 files changed, 95 insertions(+), 68 deletions(-)
>
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index 1990984..2c3e441 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -537,6 +537,11 @@ static int handle_fragments(struct net *net, struct
> sw_flow_key *key,
> return -EPFNOSUPPORT;
> }
>
> + /* The key extracted from the fragment that completed this datagram
> + * likely didn't have an L4 header, so regenerate it.
> + */
> + ovs_flow_key_update_l3l4(skb, key);
> +
> key->ip.frag = OVS_FRAG_TYPE_NONE;
> skb_clear_hash(skb);
> skb->ignore_df = 1;
> diff --git a/datapath/flow.c b/datapath/flow.c
> index 99dae79..083288f 100644
> --- a/datapath/flow.c
> +++ b/datapath/flow.c
> @@ -493,79 +493,14 @@ invalid:
> }
>
> /**
> - * key_extract - extracts a flow key from an Ethernet frame.
> + * key_extract_l3l4 - extracts L3/L4 header information.
> * @skb: sk_buff that contains the frame, with skb->data pointing to the
> - * Ethernet header
> + * L3 header
> * @key: output flow key
> - *
> - * The caller must ensure that skb->len >= ETH_HLEN.
> - *
> - * Returns 0 if successful, otherwise a negative errno value.
> - *
> - * Initializes @skb header fields as follows:
> - *
> - * - skb->mac_header: the L2 header.
> - *
> - * - skb->network_header: just past the L2 header, or just past the
> - * VLAN header, to the first byte of the L2 payload.
> - *
> - * - skb->transport_header: If key->eth.type is ETH_P_IP or ETH_P_IPV6
> - * on output, then just past the IP header, if one is present and
> - * of a correct length, otherwise the same as skb->network_header.
> - * For other key->eth.type values it is left untouched.
> - *
> - * - skb->protocol: the type of the data starting at skb->network_header.
> - * Equals to key->eth.type.
> */
> -static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
> +static int key_extract_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
> {
> int error;
> - struct ethhdr *eth;
> -
> - /* Flags are always used as part of stats */
> - key->tp.flags = 0;
> -
> - skb_reset_mac_header(skb);
> -
> - /* Link layer. */
> - clear_vlan(key);
> - if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
> - if (unlikely(eth_type_vlan(skb->protocol)))
> - return -EINVAL;
> -
> - skb_reset_network_header(skb);
> - key->eth.type = skb->protocol;
> - } else {
> - eth = eth_hdr(skb);
> - ether_addr_copy(key->eth.src, eth->h_source);
> - ether_addr_copy(key->eth.dst, eth->h_dest);
> -
> - __skb_pull(skb, 2 * ETH_ALEN);
> - /* We are going to push all headers that we pull, so no need to
> - * update skb->csum here.
> - */
> -
> - if (unlikely(parse_vlan(skb, key)))
> - return -ENOMEM;
> -
> - key->eth.type = parse_ethertype(skb);
> - if (unlikely(key->eth.type == htons(0)))
> - return -ENOMEM;
> -
> - /* Multiple tagged packets need to retain TPID to satisfy
> - * skb_vlan_pop(), which will later shift the ethertype into
> - * skb->protocol.
> - */
> - if (key->eth.cvlan.tci & htons(VLAN_TAG_PRESENT))
> - skb->protocol = key->eth.cvlan.tpid;
> - else
> - skb->protocol = key->eth.type;
> -
> - skb_reset_network_header(skb);
> - __skb_push(skb, skb->data - skb_mac_header(skb));
> - }
> -
> - skb_reset_mac_len(skb);
>
> /* Network layer. */
> if (key->eth.type == htons(ETH_P_IP)) {
> @@ -756,6 +691,92 @@ static int key_extract(struct sk_buff *skb, struct
> sw_flow_key *key)
> return 0;
> }
>
> +/**
> + * key_extract - extracts a flow key from an Ethernet frame.
> + * @skb: sk_buff that contains the frame, with skb->data pointing to the
> + * Ethernet header
> + * @key: output flow key
> + *
> + * The caller must ensure that skb->len >= ETH_HLEN.
> + *
> + * Returns 0 if successful, otherwise a negative errno value.
> + *
> + * Initializes @skb header fields as follows:
> + *
> + * - skb->mac_header: the L2 header.
> + *
> + * - skb->network_header: just past the L2 header, or just past the
> + * VLAN header, to the first byte of the L2 payload.
> + *
> + * - skb->transport_header: If key->eth.type is ETH_P_IP or ETH_P_IPV6
> + * on output, then just past the IP header, if one is present and
> + * of a correct length, otherwise the same as skb->network_header.
> + * For other key->eth.type values it is left untouched.
> + *
> + * - skb->protocol: the type of the data starting at skb->network_header.
> + * Equals to key->eth.type.
> + */
> +static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + struct ethhdr *eth;
> +
> + /* Flags are always used as part of stats */
> + key->tp.flags = 0;
> +
> + skb_reset_mac_header(skb);
> +
> + /* Link layer. */
> + clear_vlan(key);
> + if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
> + if (unlikely(eth_type_vlan(skb->protocol)))
> + return -EINVAL;
> +
> + skb_reset_network_header(skb);
> + key->eth.type = skb->protocol;
> + } else {
> + eth = eth_hdr(skb);
> + ether_addr_copy(key->eth.src, eth->h_source);
> + ether_addr_copy(key->eth.dst, eth->h_dest);
> +
> + __skb_pull(skb, 2 * ETH_ALEN);
> + /* We are going to push all headers that we pull, so no need to
> + * update skb->csum here.
> + */
> +
> + if (unlikely(parse_vlan(skb, key)))
> + return -ENOMEM;
> +
> + key->eth.type = parse_ethertype(skb);
> + if (unlikely(key->eth.type == htons(0)))
> + return -ENOMEM;
> +
> + /* Multiple tagged packets need to retain TPID to satisfy
> + * skb_vlan_pop(), which will later shift the ethertype into
> + * skb->protocol.
> + */
> + if (key->eth.cvlan.tci & htons(VLAN_CFI_MASK))
> + skb->protocol = key->eth.cvlan.tpid;
> + else
> + skb->protocol = key->eth.type;
> +
> + skb_reset_network_header(skb);
> + __skb_push(skb, skb->data - skb_mac_header(skb));
> + }
> +
> + skb_reset_mac_len(skb);
> +
> + /* Fill out L3/L4 key info, if any */
> + return key_extract_l3l4(skb, key);
> +}
> +
> +/* In the case of conntrack fragment handling it expects L3 headers,
> + * add a helper.
> + */
> +int ovs_flow_key_update_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + return key_extract_l3l4(skb, key);
> +}
> +
> int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
> {
> int res;
> diff --git a/datapath/flow.h b/datapath/flow.h
> index 07af912..78edb36 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
> @@ -275,6 +275,7 @@ u64 ovs_flow_used_time(unsigned long flow_jiffies);
>
> /* Update the non-metadata part of the flow key using skb. */
> int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key);
> +int ovs_flow_key_update_l3l4(struct sk_buff *skb, struct sw_flow_key *key);
> int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
> struct sk_buff *skb,
> struct sw_flow_key *key);
> --
> 1.8.3.1
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
