[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOrHB_COGWb78kN70QFKgA3w5v3zasAW=TJbzoE5zZYKao_GvA@mail.gmail.com>
Date: Sat, 4 Nov 2017 07:29:46 -0700
From: Pravin Shelar <pshelar@....org>
To: Yi Yang <yi.y.yang@...el.com>
Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>,
ovs dev <dev@...nvswitch.org>, Jiri Benc <jbenc@...hat.com>,
Eric Garver <e@...g.me>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next v15] openvswitch: enable NSH support
On Tue, Oct 31, 2017 at 9:03 PM, Yi Yang <yi.y.yang@...el.com> wrote:
> v14->v15
> - Check size in nsh_hdr_from_nlattr
> - Fixed four small issues pointed out By Jiri and Eric
>
> v13->v14
> - Rename skb_push_nsh to nsh_push per Dave's comment
> - Rename skb_pop_nsh to nsh_pop per Dave's comment
>
> v12->v13
> - Fix NSH header length check in set_nsh
>
> v11->v12
> - Fix missing changes old comments pointed out
> - Fix new comments for v11
>
> v10->v11
> - Fix the left three disputable comments for v9
> but not fixed in v10.
>
> v9->v10
> - Change struct ovs_key_nsh to
> struct ovs_nsh_key_base base;
> __be32 context[NSH_MD1_CONTEXT_SIZE];
> - Fix new comments for v9
>
> v8->v9
> - Fix build error reported by daily intel build
> because nsh module isn't selected by openvswitch
>
> v7->v8
> - Rework nested value and mask for OVS_KEY_ATTR_NSH
> - Change pop_nsh to adapt to nsh kernel module
> - Fix many issues per comments from Jiri Benc
>
> v6->v7
> - Remove NSH GSO patches in v6 because Jiri Benc
> reworked it as another patch series and they have
> been merged.
> - Change it to adapt to nsh kernel module added by NSH
> GSO patch series
>
> v5->v6
> - Fix the rest comments for v4.
> - Add NSH GSO support for VxLAN-gpe + NSH and
> Eth + NSH.
>
> v4->v5
> - Fix many comments by Jiri Benc and Eric Garver
> for v4.
>
> v3->v4
> - Add new NSH match field ttl
> - Update NSH header to the latest format
> which will be final format and won't change
> per its author's confirmation.
> - Fix comments for v3.
>
> v2->v3
> - Change OVS_KEY_ATTR_NSH to nested key to handle
> length-fixed attributes and length-variable
> attriubte more flexibly.
> - Remove struct ovs_action_push_nsh completely
> - Add code to handle nested attribute for SET_MASKED
> - Change PUSH_NSH to use the nested OVS_KEY_ATTR_NSH
> to transfer NSH header data.
> - Fix comments and coding style issues by Jiri and Eric
>
> v1->v2
> - Change encap_nsh and decap_nsh to push_nsh and pop_nsh
> - Dynamically allocate struct ovs_action_push_nsh for
> length-variable metadata.
>
> OVS master and 2.8 branch has merged NSH userspace
> patch series, this patch is to enable NSH support
> in kernel data path in order that OVS can support
> NSH in compat mode by porting this.
>
> Signed-off-by: Yi Yang <yi.y.yang@...el.com>
As commented earlier following are action related validations that can
be moved to flow install phase.
> ---
> include/net/nsh.h | 3 +
> include/uapi/linux/openvswitch.h | 29 ++++
> net/nsh/nsh.c | 59 ++++++++
> net/openvswitch/Kconfig | 1 +
> net/openvswitch/actions.c | 119 +++++++++++++++
> net/openvswitch/flow.c | 51 +++++++
> net/openvswitch/flow.h | 7 +
> net/openvswitch/flow_netlink.c | 315 ++++++++++++++++++++++++++++++++++++++-
> net/openvswitch/flow_netlink.h | 5 +
> 9 files changed, 588 insertions(+), 1 deletion(-)
>
...
> diff --git a/net/nsh/nsh.c b/net/nsh/nsh.c
> index 58fb827..2764682 100644
> --- a/net/nsh/nsh.c
> +++ b/net/nsh/nsh.c
> @@ -14,6 +14,65 @@
> #include <net/nsh.h>
> #include <net/tun_proto.h>
>
> +int nsh_push(struct sk_buff *skb, const struct nshhdr *pushed_nh)
> +{
> + struct nshhdr *nh;
> + size_t length = nsh_hdr_len(pushed_nh);
> + u8 next_proto;
> +
> + if (skb->mac_len) {
> + next_proto = TUN_P_ETHERNET;
> + } else {
> + next_proto = tun_p_from_eth_p(skb->protocol);
> + if (!next_proto)
> + return -EAFNOSUPPORT;
check for supported protocols can be moved to flow install validation
in __ovs_nla_copy_actions().
> + }
> +
> + /* Add the NSH header */
> + if (skb_cow_head(skb, length) < 0)
> + return -ENOMEM;
> +
> + skb_push(skb, length);
> + nh = (struct nshhdr *)(skb->data);
> + memcpy(nh, pushed_nh, length);
> + nh->np = next_proto;
> +
> + skb->protocol = htons(ETH_P_NSH);
> + skb_reset_mac_header(skb);
> + skb_reset_network_header(skb);
> + skb_reset_mac_len(skb);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(nsh_push);
> +
> +int nsh_pop(struct sk_buff *skb)
> +{
> + struct nshhdr *nh;
> + size_t length;
> + __be16 inner_proto;
> +
> + if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))
> + return -ENOMEM;
> + nh = (struct nshhdr *)(skb->data);
> + length = nsh_hdr_len(nh);
> + inner_proto = tun_p_to_eth_p(nh->np);
same as above, this check can be moved to flow install __ovs_nla_copy_actions().
> + if (!pskb_may_pull(skb, length))
> + return -ENOMEM;
> +
> + if (!inner_proto)
> + return -EAFNOSUPPORT;
> +
> + skb_pull(skb, length);
> + skb_reset_mac_header(skb);
> + skb_reset_network_header(skb);
> + skb_reset_mac_len(skb);
> + skb->protocol = inner_proto;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(nsh_pop);
> +
...
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index a551232..dd1449d 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
...
> +static int pop_nsh(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> + int err;
> +
> + if (ovs_key_mac_proto(key) != MAC_PROTO_NONE ||
> + skb->protocol != htons(ETH_P_NSH)) {
> + return -EINVAL;
> + }
> +
These checks can be moved to flow install.
> + err = nsh_pop(skb);
> + if (err)
> + return err;
> +
> + /* safe right before invalidate_flow_key */
> + if (skb->protocol == htons(ETH_P_TEB))
> + key->mac_proto = MAC_PROTO_ETHERNET;
> + else
> + key->mac_proto = MAC_PROTO_NONE;
> + invalidate_flow_key(key);
> + return 0;
> +}
> +
Powered by blists - more mailing lists