netdev - Re: [PATCH net-next v15] openvswitch: enable NSH support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOrHB_DKEHXYMkU+h+OiKsQ2Qh4VTJvVCD33ek4465rE0yDbAw@mail.gmail.com>
Date:   Mon, 6 Nov 2017 04:09:53 -0800
From:   Pravin Shelar <pshelar@....org>
To:     "Yang, Yi" <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 Sun, Nov 5, 2017 at 8:19 PM, Yang, Yi <yi.y.yang@...el.com> wrote:
> On Sat, Nov 04, 2017 at 10:29:46PM +0800, Pravin Shelar wrote:
>> On Tue, Oct 31, 2017 at 9:03 PM, Yi Yang <yi.y.yang@...el.com> wrote:
>> > +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().
>
> Pravin, these two functions are not only for OVS, you can see it is
> net/nsh/nsh.c, Jiri and Eric mentioned they also could be used by TC.
>
I think it can be easily done by other caller of these function, or
you can refactor the APIs itself.

> I understand you expect some checks should be moved to slow path, but
> for there two cases, we can't remove them into __ovs_nla_copy_actions.
>
It is not just about optimization, but having these check in flow
install allows OVS userspace to probe level of  NSH support in
datapath.