Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

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

On 2013年12月20日 17:04, Pablo Neira Ayuso wrote:
> On Thu, Dec 19, 2013 at 11:30:09AM +0800, Fan Du wrote:
>>
>>
>> On 2013年12月17日 21:05, Pablo Neira Ayuso wrote:
>>> On Fri, Dec 13, 2013 at 08:18:00PM +0800, Fan Du wrote:
>>>> Hi,
>>>>
>>>> This patchset adds IPv4/v6 IPComp 'match' plugin to enables user setting
>>>> ACTONs for IPcomp flows sepecified with SPI value.
>>>>
>>>> Corresponding iptables patchset will be sent here after soon.
>>>>
>>>> Fan Du (2):
>>>>    netfilter: add IPv4 IPComp extension match support
>>>>    netfilter: add IPv6 IPComp extension match support
>>>
>>> This looks good, but I have to ask you to merge those two modules into
>>> one single xt_ipcomp, they are fairly small and we can save the
>>> overhead of having two different modules. Moreover, at quick glance I
>>> don't see any dependency with IPv4/IPv6 exported symbols that may
>>> cause ifdef pollution.
>>>
>>> Please, see net/netfilter/xt_tcpudp.c as reference to rework this.
>>> Thanks.
>>>
>>
>> I noticed netfilter ipv4/v6 AH support also has split implementation,
>> so far, by my understanding, it's fairly enough to consolidate those
>> two implementations into one as well, as IPv4/6 AH head format are
>> identical.
>>
>> If you don't mind or it won't break anything internal for netfilter,
>> I plan to combine them into one piece.
>
> We can merge those two, but you'll have to handle dependencies with
> IPv6 core via ifdefs and Kconfig to avoid problems.
>
> AH is not the last header, so we still have to use ipv6_find_hdr() to
> find the good header instead of par->thoff. Note that the ip6_tables
> sets par->thoff to the last IPv6 extension header.

I'm quite new to the internal of netfiler, especially about this part.
I will take a look at the code later.

> This rises some concerns regarding your ipcomp, I think that if you
> use this with ah and esp, the ordering of the headers is
> ah+ipcomp+esp, right?

This depends on the user land configuration of encapsulation order.
It can be one of the three types only(ah, esp, ipcomp), the most commonly
used is ah(outer)+esp(inner).

I barely see ipcomp used in production, but I remember RFC says ipcomp
should be done first before esp, because after encryption in esp, the data
is polluted, i.e., not suitable for compressed anymore(I'm not sure the
details theory behind this statement.)

Anyway, no matter how ah,esp,ipcomp are ordered, I think it only makes
sense for the outer header when using any one of those three filters, and
that's what I can imagine how it's been used.

-- 
浮沉随浪只记今朝笑

--fan