netdev - Re: [PATCH net-next RFC v2 1/3] lwt: infrastructure to support light weight tunnels

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: <558431C3.5020703@cumulusnetworks.com>
Date:	Fri, 19 Jun 2015 08:14:11 -0700
From:	roopa <roopa@...ulusnetworks.com>
To:	Robert Shearman <rshearma@...cade.com>
CC:	ebiederm@...ssion.com, tgraf@...g.ch, davem@...emloft.net,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next RFC v2 1/3] lwt: infrastructure to support light
 weight tunnels

On 6/19/15, 7:43 AM, Robert Shearman wrote:
>> diff --git a/include/linux/lwtunnel.h b/include/linux/lwtunnel.h
>> new file mode 100644 
<snip>
>> +/* lw tunnel state flags */
>> +#define LWTUNNEL_STATE_OUTPUT_REDIRECT 0x1
>> +
>> +#define lwtunnel_output_redirect(lwtstate) (lwtstate && \
>> +            (lwtstate->flags & LWTUNNEL_STATE_OUTPUT_REDIRECT))
>
> This could be made an inline function for type-safety.
ack
>
>> +
>> +struct lwtunnel_state {
>> +    __u16        type;
>> +    __u16        flags;
>> +    atomic_t    refcnt;
>> +    struct lwtunnel_hdr tunnel;
>> +};
>> +
>> +struct lwtunnel_net {
>> +    struct hlist_head tunnels[LWTUNNEL_HASH_SIZE];
>> +};
>
> This type doesn't appear to be used in this patch series. Do you 
> intend to use it in a future version?
ack, will get rid of it
>
>>
>> +
>> +static inline struct lwtunnel_state *lwtunnel_skb_lwstate(struct 
>> sk_buff *skb)
>> +{
>> +    struct rtable *rt = (struct rtable *)skb_dst(skb);
>> +
>> +    return rt->rt_lwtstate;
>> +}
>
> It doesn't look like this patch will build on its own because 
> rt_lwtstate isn't added to struct rtable until patch 2.
looks like i messed up the patch creation. I will fix that with the next 
series.
>
> More importantly, is it safe to assume that skb_dst will always return 
> an IPv4 dst? How will this look when IPv6 support is added?

Today lwtunnel_skb_lwstate is called from lwtunnel_output which is only 
called from ipv4 code.
And my ipv6 variant code was supposed to have a 6 suffix. something like 
lwtunnel_output6.
Or to be more explicit i will probably have variants of the output and 
skb handling functions like,
lwtunnel_output_ipv4 and lwtunnel_output_ipv6.
>> +
>> +    ret = -EOPNOTSUPP;
>> +    nest = nla_nest_start(skb, RTA_ENCAP);
>
> Again, it doesn't look like this will build since RTA_ENCAP isn't 
> added until patch 2.
>
ack, sorry abt the patch ordering. will fix it.

Thanks for the review.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in