netdev - Re: [PATCH net-next v12] 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: <20171019131215.GA84551@cran64.bj.intel.com>
Date:   Thu, 19 Oct 2017 21:12:15 +0800
From:   "Yang, Yi" <yi.y.yang@...el.com>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "dev@...nvswitch.org" <dev@...nvswitch.org>,
        "e@...g.me" <e@...g.me>, "pshelar@....org" <pshelar@....org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH net-next v12] openvswitch: enable NSH support

On Thu, Oct 19, 2017 at 02:43:47PM +0200, Jiri Benc wrote:
> On Thu, 19 Oct 2017 19:40:53 +0800, Yang, Yi wrote:
> > Actually mdtype can't be set, only push_nsh can set mdtype, so set_nsh
> > won't have mdtype flow key, we can't get it from flow_key in set_nsh,
> > only ttl, flags and path_hdr can be set in set_nsh as you can see in code.
> > I understand your concern is calling skb_ensure_writable twice, so
> > changing the first one to pskb_may_pull is more appropriate for set_nsh.
> 
> Isn't set_nsh called only after the packet was already dissected (i.e.
> parse_nsh called)? The dissected fields should be available in flow_key.

flow_key in set_nsh is got from netlink message which is set by
commit_nsh in user space, here is code.

    mask.mdtype = 0;     /* Not writable. */
    mask.np = 0;         /* Not writable. */

    if (commit_nsh(&base_flow->nsh, use_masked, &key, &base, &mask,
            sizeof key, odp_actions)) {
        put_nsh_key(&base, base_flow, false);
        if (mask.mdtype != 0) { /* Mask was changed by commit(). */
            put_nsh_key(&mask, &wc->masks, true);
        }
    }

Here set is set with mask, so key value is masked by mask.mdtype

        /* OVS_KEY_ATTR_NSH keys */
        nsh_key_ofs = nl_msg_start_nested(odp_actions,
OVS_KEY_ATTR_NSH);

        /* put value and mask for OVS_NSH_KEY_ATTR_BASE */
        char *data = nl_msg_put_unspec_uninit(odp_actions,
                                              OVS_NSH_KEY_ATTR_BASE,
                                              2 * sizeof(nsh_base));
        const char *lkey = (char *)&nsh_base, *lmask = (char
*)&nsh_base_mask;
        size_t lkey_size = sizeof(nsh_base);

        while (lkey_size--) {
            *data++ = *lkey++ & *lmask++;
        }
        lmask = (char *)&nsh_base_mask;
        memcpy(data, lmask, sizeof(nsh_base_mask));

> 
>  Jiri