netdev - Re: [PATCH net-next 2/9] gue: Support for Generic UDP Encapsulation RX path

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] [day] [month] [year] [list]

Message-ID: <CAJZOPZLcMRbqm=TGGWaP+VS9HOCPNfeq6_X_9Uzn6hn-f2JEiA@mail.gmail.com>
Date:	Fri, 25 Jul 2014 22:18:04 +0300
From:	Or Gerlitz <or.gerlitz@...il.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/9] gue: Support for Generic UDP Encapsulation
 RX path

On Thu, Jul 24, 2014 at 8:28 PM, Tom Herbert <therbert@...gle.com> wrote:
> On Thu, Jul 24, 2014 at 12:07 AM, Or Gerlitz <or.gerlitz@...il.com> wrote:
>>
>> On Thu, Jul 17, 2014 at 7:14 AM, Tom Herbert <therbert@...gle.com> wrote:
>> > This patch provides a Generic UDP Encapsulation receive path
>> > using the XFRM framework (udp_encap_rcv).
>> >
>> > This adds support for both direct encapsulation of IP protocols over
>> > UDP. The bound destination port is used to map to an IP protocol.
>> > This should support GRE over UDP encapsulation, see
>> > http://tools.ietf.org/html/draft-yong-tsvwg-gre-in-udp-encap-02,
>> > as will as the other IP tunneling protocols (IPIP, SIT).

>> > +++ b/include/net/gue.h
>> > @@ -0,0 +1,49 @@
>> > +#ifndef __NET_GUE_H
>> > +#define __NET_GUE_H
>> > +
>> > +#if defined(CONFIG_NET_GUE) || defined(CONFIG_NET_GUE_MODULE)
>> > +
>> > +#include <net/udp_tunnel.h>
>> > +
>> > +struct gue_port_cfg {
>> > +       u8                      protocol;
>> > +       struct udp_port_cfg     udp_config;
>> > +};
>> > +
>> > +int gue_port_create(struct net *net, struct gue_port_cfg *cfg,
>> > +                   struct socket **sockp);
>> > +int gue_open_direct_port(unsigned short port, unsigned char proto,
>> > +                        struct socket **sockp);
>> > +void gue_close_port(struct socket *sock);
>> > +
>> > +/* Tunnel protocols can use GUE_TUNNEL_PARMS to set up per module static
>> > + * variables that are used with the common GUE functions (open_gue_port,
>> > + * close_gue_port).
>> > + */
>> > +#define GUE_TUNNEL_PARMS(name, default_port)                   \
>> > +static int gue_udp_port = default_port;                                \
>> > +module_param(gue_udp_port, int, 0);                            \
>> > +MODULE_PARM_DESC(gue_udp_port, name "/UDP port");              \
>> > +static struct socket *gue_sock;
>> > +
>> > +#define open_gue_port(proto)                                   \
>> > +       (gue_udp_port ?                                         \
>> > +        gue_open_direct_port(gue_udp_port, proto, &gue_sock) : \
>> > +        0)
>> > +
>> > +#define close_gue_port() do {                                  \
>> > +       if (gue_sock) {                                         \
>> > +               gue_close_port(gue_sock);                       \
>> > +               gue_sock = NULL;                                \
>> > +       }                                                       \
>> > +} while (0)
>> > +

>> So this is probably a similar feedback to the ioctls vs netlink
>> comment from Dave...
>> I guess we agree that module params should be avoided when possible, right? so
>> this even goes further to a template in a header file used by the
>> caller modules to define
>> their own module param...  should be (a non academic, I read your other comment)
>> alternative here, I hope.

> I'm not sure. The gue_port is a global property of the module and not
> link or device specific.

I wonder what makes the GUE port a global property eg. vs. VXLAN where
people expressed thier will and AFAIK showed some valid use cases
where multiple ports could be used?


>  My precedence is vxlan driver which has udp_port module parameter.

You can now perfectly set the udp port for vxlan through netlink for
both the bridge and OVS schemes. The fact that initially it was set
through a module param doesn't justify to repeat this habbit again...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html