On 03/11/2018 10:14 PM, Pablo Neira Ayuso wrote:
> On Sun, Mar 04, 2018 at 09:28:53AM +0100, Matthias Schiffer wrote:
>> We already have ICMPv6 type/code matches. This adds support for IPv4 ICMP
>> matches in the same way.
>>
>> Signed-off-by: Matthias Schiffer <mschif...@universe-factory.net>
>> ---
>>  include/uapi/linux/netfilter_bridge/ebt_ip.h | 13 +++++++--
>>  net/bridge/netfilter/ebt_ip.c                | 43 
>> +++++++++++++++++++++-------
>>  2 files changed, 43 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/uapi/linux/netfilter_bridge/ebt_ip.h 
>> b/include/uapi/linux/netfilter_bridge/ebt_ip.h
>> index 8e462fb1983f..4ed7fbb0a482 100644
>> --- a/include/uapi/linux/netfilter_bridge/ebt_ip.h
>> +++ b/include/uapi/linux/netfilter_bridge/ebt_ip.h
>> @@ -24,8 +24,9 @@
>>  #define EBT_IP_PROTO 0x08
>>  #define EBT_IP_SPORT 0x10
>>  #define EBT_IP_DPORT 0x20
>> +#define EBT_IP_ICMP 0x40
>>  #define EBT_IP_MASK (EBT_IP_SOURCE | EBT_IP_DEST | EBT_IP_TOS | 
>> EBT_IP_PROTO |\
>> - EBT_IP_SPORT | EBT_IP_DPORT )
>> +                 EBT_IP_SPORT | EBT_IP_DPORT | EBT_IP_ICMP)
>>  #define EBT_IP_MATCH "ip"
>>  
>>  /* the same values are used for the invflags */
>> @@ -38,8 +39,14 @@ struct ebt_ip_info {
>>      __u8  protocol;
>>      __u8  bitmask;
>>      __u8  invflags;
>> -    __u16 sport[2];
>> -    __u16 dport[2];
>> +    union {
>> +            __u16 sport[2];
>> +            __u8 icmp_type[2];
>> +    };
>> +    union {
>> +            __u16 dport[2];
>> +            __u8 icmp_code[2];
>> +    };
> 
> This is part of uapi, we cannot update struct ebt_ip_info, this break
> binary compatibility.

I carefully extended the struct in a way that does not change API or ABI,
in the same way the corresponding ICMPv6 matches were added in 6faee60a4e
"netfilter: ebt_ip6: allow matching on ipv6-icmp types/codes".

> 
>>  };
>>  
>>  #endif
>> diff --git a/net/bridge/netfilter/ebt_ip.c b/net/bridge/netfilter/ebt_ip.c
>> index 2b46c50abce0..8cb8f8395768 100644
>> --- a/net/bridge/netfilter/ebt_ip.c
>> +++ b/net/bridge/netfilter/ebt_ip.c
> 
> Please, place these new matching features into
> net/bridge/netfilter/ebt_ip.c, please add then new ebt_xyz.c extension
> instead.

This would increase asymmetry between ebt_ip and ebt_ip6, rather than
giving ebt_ip the same features that ebt_ip6 already had. Is there any good
reason to make it a new extension, when there is no problem to extend
ebt_ip without changing UAPI/ABI?

Regards,
Matthias

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to