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 <mschiffer@xxxxxxxxxxxxxxxxxxxx>
>> ---
>> 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
