On Fri, Oct 23, 2015 at 11:22 AM, Jiri Benc <jb...@redhat.com> wrote:
> On Fri, 23 Oct 2015 10:30:24 -0700, Pravin Shelar wrote:
...
>
>> > This doesn't do what the name suggests and is, actually, ovs specific.
>> > The ip_tunnel_info can be provided as a part of lwtstate and this
>> > function should handle that case, too. This is not a problem for
>> > net.git, as the function just returns EINVAL in such case, but should
>> > be addressed for net-next.git. As ovs is currently the only user, I'd
>> > be also fine with just a comment stating that, so it's clear for future
>> > users of this function that it needs to be extended before it can be
>> > used out of ovs.
>> >
>>
>> I considered lwstate, but I am reluctant to add this complexity
>> without a usecase.
>
> I don't think it's complex. See the quick and dirty patch below (that
> has even a negative diffstat):
>
> diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
> index ce009710120c..3a1d3966a79e 100644
> --- a/include/net/dst_metadata.h
> +++ b/include/net/dst_metadata.h
> @@ -60,36 +60,25 @@ static inline struct metadata_dst *tun_rx_dst(int md_size)
> return tun_dst;
> }
>
> -static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb)
> +static inline struct ip_tunnel_info *skb_tunnel_info_unclone(struct sk_buff
> *skb)
> {
> - struct metadata_dst *md_dst = skb_metadata_dst(skb);
> - int md_size = md_dst->u.tun_info.options_len;
> + struct metadata_dst *dst;
> + struct ip_tunnel_info *info = skb_tunnel_info(skb);
> struct metadata_dst *new_md;
>
> - if (!md_dst)
> + if (!info)
> return ERR_PTR(-EINVAL);
> -
> - new_md = metadata_dst_alloc(md_size, GFP_ATOMIC);
> + new_md = metadata_dst_alloc(info->options_len, GFP_ATOMIC);
> if (!new_md)
> return ERR_PTR(-ENOMEM);
>
> - memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
> - sizeof(struct ip_tunnel_info) + md_size);
> + memcpy(&new_md->u.tun_info, info,
> + sizeof(struct ip_tunnel_info) + info->options_len);
> skb_dst_drop(skb);
> dst_hold(&new_md->dst);
> skb_dst_set(skb, &new_md->dst);
> - return new_md;
> -}
> -
This is not complete code. I found couple of issues with it.
This code does not copy lwtunnel_state state into new dst.
And it is converting lwtunnel dst into metadata dst.
> -static inline struct ip_tunnel_info *skb_tunnel_info_unclone(struct sk_buff
> *skb)
> -{
> - struct metadata_dst *dst;
> -
> - dst = tun_dst_unclone(skb);
> - if (IS_ERR(dst))
> - return NULL;
>
> - return &dst->u.tun_info;
> + return info;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
