Subject: Re: [PATCH 6/23] [PATCH] [XFRM] STATE: Search by address using source address list.

From: Masahide NAKAMURA <[email-masked]>
Date: Wed, 02 Aug 2006 11:04:36 +0900

David Miller wrote:
> From: Masahide NAKAMURA <[email-masked]>
> Date: Sat, 29 Jul 2006 18:30:18 +0900
> 
>> +#ifdef CONFIG_XFRM_ADVANCED
>> +	struct xfrm_state	*(*state_lookup_byaddr)(xfrm_address_t *daddr, xfrm_address_t *saddr, u8 proto);
>> +#endif
> 
> I think we should delete XFRM_ADVANCED config option, it is only
> serving to make the code very ugly and full of ifdefs.

I see, I remove it.

My thought was to keep XFRM code intact as possible for users who wants only IPsec.
But don't worry, it should not be changed existing logic of course then I've made
regression test for IPsec behavior with both case: new config is off and on.
Then I guess I can remove the config easily.

>> +#ifdef CONFIG_XFRM_ADVANCED
>> +	int use_spi = xfrm_id_proto_match(x->id.proto, IPSEC_PROTO_ANY);
>> +#else
>> +	int use_spi = 1;
>> +#endif
> 
> Why this transformation?  In fact, you added several others.
> xfrm_id_proto_match() with userproto argument IPSEC_PROTO_ANY
> always evaluates to one.
> 
> If you intend to change the userproto argument in some future
> changeset, then add the xfrm_id_proto_match() call in that
> changeset.

You already find out by another mail, but I should have to change
patch order to get it easily for others or write more particular commit log.

Thanks,

-- 
Masahide NAKAMURA