Re: [DISCUSSION] rt6i_genid

[Fan Du <fan.du@windriver.com>]

On 2013年07月18日 23:12, Nicolas Dichtel wrote:
> Le 18/07/2013 11:28, Fan Du a écrit :
>>
>> Thanks for replying :)
>>
>> On 2013年07月18日 17:13, Nicolas Dichtel wrote:
>>> Le 18/07/2013 05:22, Fan Du a écrit :
>>>> Hello Nicolas
>>>>
>>>> Commit 6f3118b571b8a4c06c7985dc3172c3526cb86253: "ipv6: use net->rt_genid to
>>>> check dst validity"
>>>> makes ip6_dst_check to check rt6i_genid against with struct net->rt_genid,
>>>> As a matter of fact, struct net->rt_genid could only be modified by two places,
>>>> first is adding/delete IPv4 address, second is inserting new XFRM policy.
>>>>
>>>> Is there any other considerations that adding/deleting IPv4 address would
>>>> invalid all IPv6 dst
>>>> as well? because I'm working a patch which actually depends on the result of
>>>> this question.
>>> No, the goal was to cover the IPsec case, ie invalidate dst entries when an
>>> xfrm policy is inserted/deleted.
>>
>> Ok, then how about we only checking rt6i_genid against rt_genid *only*
>> when XFRM is enabled for IPv6, because when XFRM is not enabled for IPv6
>> ip6_dst_check for rt_genid is really not necessary.
>>
>> So what do you think of below modifications?
> Seems good. Just a small comment below.

Will send v2 for your reviewing when net-next is reopen.

Thanks

>>
>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>> index 2a601e7..4ae35fd 100644
>> --- a/include/net/ip6_fib.h
>> +++ b/include/net/ip6_fib.h
>> @@ -119,7 +119,9 @@ struct rt6_info {
>> struct inet6_dev *rt6i_idev;
>> unsigned long _rt6i_peer;
>>
>> +#ifdef CONFIG_XFRM
>> u32 rt6i_genid;
>> +#endif
>>
>> /* more non-fragment space at head required */
>> unsigned short rt6i_nfheader_len;
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index bd5fd70..1b64406 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -277,7 +277,9 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
>>
>> memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
>> rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
>> +#ifdef CONFIG_XFRM
>> rt->rt6i_genid = rt_genid(net);
>> +#endif
>> INIT_LIST_HEAD(&rt->rt6i_siblings);
>> rt->rt6i_nsiblings = 0;
>> }
>> @@ -1041,12 +1043,15 @@ static struct dst_entry *ip6_dst_check(struct dst_entry
>> *dst, u32 cookie)
>>
>> rt = (struct rt6_info *) dst;
>>
>> +#ifdef CONFIG_XFRM
>> /* All IPV6 dsts are created with ->obsolete set to the value
>> * DST_OBSOLETE_FORCE_CHK which forces validation calls down
>> * into this function always.
>> + * Note: for IPv6, rt6i_genid is noly used when XFRM enabled.
> indentation is wrong, but I'm not sure this comment is really needed ... you've put the ifdef 4 lines above.
>
>> */
>> if (rt->rt6i_genid != rt_genid(dev_net(rt->dst.dev)))
>> return NULL;
>> +#endif
>>
>> if (rt->rt6i_node && (rt->rt6i_node->fn_sernum == cookie))
>> return dst;
>

-- 
浮沉随浪只记今朝笑

--fan