netdev - Re: [PATCH net-next v6 3/3] 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: <20170904023831.GA68062@cran64.bj.intel.com>
Date:   Mon, 4 Sep 2017 10:38:32 +0800
From:   "Yang, Yi" <yi.y.yang@...el.com>
To:     Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "dev@...nvswitch.org" <dev@...nvswitch.org>,
        "jbenc@...hat.com" <jbenc@...hat.com>, "e@...g.me" <e@...g.me>,
        "blp@....org" <blp@....org>,
        "jan.scheurich@...csson.com" <jan.scheurich@...csson.com>
Subject: Re: [PATCH net-next v6 3/3] openvswitch: enable NSH support

On Wed, Aug 30, 2017 at 05:53:27PM +0800, Hannes Frederic Sowa wrote:
> Hello,
> 
> Yi Yang <yi.y.yang@...el.com> writes:
> 
> [...]
> 
> > +struct ovs_key_nsh {
> > +	u8 flags;
> > +	u8 ttl;
> > +	u8 mdtype;
> > +	u8 np;
> > +	__be32 path_hdr;
> > +	__be32 context[NSH_MD1_CONTEXT_SIZE];
> > +};
> > +
> >  struct sw_flow_key {
> >  	u8 tun_opts[IP_TUNNEL_OPTS_MAX];
> >  	u8 tun_opts_len;
> > @@ -144,6 +154,7 @@ struct sw_flow_key {
> >  			};
> >  		} ipv6;
> >  	};
> > +	struct ovs_key_nsh nsh;         /* network service header */
> >  	struct {
> >  		/* Connection tracking fields not packed above. */
> >  		struct {
> 
> Does it makes sense to keep the context headers as part of the flow?
> What is the reasoning behind it? With mdtype 2 headers this might either
> not work very well or will increase sw_flow_key size causing slowdowns
> for all protocols.

For userspace, miniflow can handle such issue, but kernel data path
didn't provide such mechanism, so I don't think we can think of a better
way to fix this.

We have to have context headers in flow for match and set, every hop in
service function chaining can put its metedata into context headers on
demand, MD type 2 is much more complicated than you can imagine, it is
impossible to use an uniform way to handle MD type 1 and MD type 2, our
way is to keep MD type 1 keys in struct ovs_key_nsh and to reuse struct
flow_tnl for MD type 2 metadata, in case of MD type 2, we can set
context headers to 0 in struct ovs_key_nsh.

I beleive every newly-added key will result in similiar issue you
concern, maybe it will be better to introduce miniflow in kernel data
path, but it isn't in scope of this patch series.

It will be great if you can have a better proposal to fix your concern.

> 
> [...]