Re: [PATCH net-next] xfrm: Simplify SA looking up when using wildcard source address

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

On 2013年09月24日 19:45, Steffen Klassert wrote:
> On Mon, Sep 23, 2013 at 05:18:37PM +0800, Fan Du wrote:
>> I'm not quite sure I get this "wildcard source address" right,
>> IMHO if a host needs to protect every traffic for a given remote host,
>> then the source address is wildcard address, i.e. all ZEROs.
>> (Please correct me if I'm bloodly wrong。。。)
>
> The above does not belong to a commit message, really.
> If you are not sure and you want comments on your patch,
> mark your patch as RFC. You should be sure that your patch
> is correct when you submit, at least in the moment you
> send it. I know that this can change a second after,
> but in that moment you should be sure.

One day without embarrassment is not my day :)
Have sent v2, please kindly review.

Thanks

>>
>> Here is the argument if above statement stands true:
>> __xfrm4/6_state_addr_check is a four steps check, all we need to do
>> is checking whether the destination address match. Passing saddr from
>> flow is worst option, as the checking needs to reach the fourth step.
>>
>> So, simply this process by only checking destination address only when
>> using wildcard source address for looking up SAs.
>>
>> Signed-off-by: Fan Du<fan.du@windriver.com>
>> ---
>
> If you have further comments on your patch that should not be
> included in the commit message, you can add them here.
>
>>   include/net/xfrm.h    |   31 +++++++++++++++++++++++++++++++
>>   net/xfrm/xfrm_state.c |    2 +-
>>   2 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
>> index e253bf0..fdb9343 100644
>> --- a/include/net/xfrm.h
>> +++ b/include/net/xfrm.h
>> @@ -1282,6 +1282,37 @@ xfrm_state_addr_check(const struct xfrm_state *x,
>>   }
>>
>>   static __inline__ int
>> +__xfrm4_state_daddr_check(const struct xfrm_state *x,
>> +                                const xfrm_address_t *daddr)
>> +{
>> +        return ((daddr->a4 == x->id.daddr.a4) ? 1 : 0);
>> +}
>> +
>> +static __inline__ int
>> +__xfrm6_state_daddr_check(const struct xfrm_state *x,
>> +                         const xfrm_address_t *daddr)
>> +{
>> +        if (ipv6_addr_equal((struct in6_addr *)daddr, (struct in6_addr *)&x->id.daddr))
>> +                return 1;
>> +        else
>> +                return 0;
>> +}
>> +
>> +static __inline__ int
>> +xfrm_state_daddr_check(const struct xfrm_state *x,
>> +                      const xfrm_address_t *daddr,
>> +                      unsigned short family)
>> +{
>> +        switch (family) {
>> +        case AF_INET:
>> +                return __xfrm4_state_daddr_check(x, daddr);
>> +        case AF_INET6:
>> +                return __xfrm6_state_daddr_check(x, daddr);
>> +        }
>> +        return 0;
>> +}
>
> You used whitespaces where you should use tabs in the whole patch.
> Please do the formating right to avoid cleanup patches.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
浮沉随浪只记今朝笑

--fan