netdev - Re: [RFC 1/9] ipv6: implement dataplane support for rthdr type 4 (Segment Routing Header)

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: <57C828EC.60501@uclouvain.be>
Date:   Thu, 1 Sep 2016 15:11:08 +0200
From:   David Lebrun <david.lebrun@...ouvain.be>
To:     <nicolas.dichtel@...nd.com>, <netdev@...r.kernel.org>
Subject: Re: [RFC 1/9] ipv6: implement dataplane support for rthdr type 4
 (Segment Routing Header)

On 08/31/2016 04:51 PM, Nicolas Dichtel wrote:
> Thanks for proposing this feature. It would be great to have it upstream.
> 

Thanks for the feedback :)

> [snip]
>> +config IPV6_SEG6
>> +	bool "IPv6: Segment Routing support"
>> +	depends on IPV6
>> +	---help---
>> +	  Experimental support for IPv6 Segment Routing dataplane as defined
>> +	  in IETF draft-ietf-6man-segment-routing-header-01. This option
>> +	  enables the processing of SR-enabled packets allowing the kernel
>> +	  to act as a segment endpoint (intermediate or egress).
>> +
>> +	  If unsure, say N.
>> +
> I don't think that the option is needed. At the end, every distributions will
> turn it on.
> 

Are you sure ? This is a rather specific feature, used in specific
environments. Not that I would mind removing the option if it makes sense.

> [snip]
>> +#ifdef CONFIG_IPV6_SEG6
>> +	{
>> +		.procname	= "seg6_enabled",
>> +		.data		= &ipv6_devconf.seg6_enabled,
>> +		.maxlen		= sizeof(int),
>> +		.mode		= 0644,
>> +		.proc_handler	= proc_dointvec,
>> +	},
>> +#endif
> Don't forget to document this option in Documentation/networking/ip-sysctl.txt.
> Don't forget to explain how 'all' works ;-)
> It would be nice to also add it in netconf subsystem (see 'git grep netconf
> net/ipv6').
> 

Right ! I didn't think of that doc file. Noted for netconf.

> [snip]
>> +#ifdef CONFIG_IPV6_SEG6
>> +static int ipv6_srh_rcv(struct sk_buff *skb)
>> +{
>> +	struct inet6_skb_parm *opt = IP6CB(skb);
>> +	struct in6_addr *addr = NULL, *last_addr = NULL, *active_addr = NULL;
>> +	struct ipv6_sr_hdr *hdr;
>> +	struct net *net = dev_net(skb->dev);
>> +	int cleanup = 0;
>> +	struct inet6_dev *idev;
>> +	int accept_seg6;
> nit: better to follow the 'reverse christmas tree' scheme when declaring variables.
> 

Noted


>> +
>> +	ip6_route_input(skb);
> The destination address has now changed and the packet is routed again.
> skb->nfct is not updated, it is intentional? I'm asking me if it's conceptually
> right.
> 

I fail to see any usecase where conntrack would run on SR-enabled
packets. Things such as NAT would just defeat the purpose.

David


Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)