Re: [PATCH 1/3] Add revision field for xt_entry_target

Re: [PATCH 1/3] Add revision field for xt_entry_target

[Zhang, Chunyu]

hi Pablo

>On Fri, Feb 06, 2015 at 03:26:28PM +0800, Gao feng wrote:
>> This filed is useful if we want to add TARGET which
>> has revision for arptables rules.
>>
>> Also make sure xt_entry_target is consistent with
>> the definition in kernel.
>>
>> Signed-off-by: Gao feng <gaofeng@xxxxxxxxxxxxxx>
>> ---
>>  include/linux/netfilter_arp/arp_tables.h | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h
>> index 0acda66..ccf8cd0 100644
>> --- a/include/linux/netfilter_arp/arp_tables.h
>> +++ b/include/linux/netfilter_arp/arp_tables.h
>> @@ -19,7 +19,7 @@
>> 
>>  #include <linux/netfilter_arp.h>
>> 
>> -#define ARPT_FUNCTION_MAXNAMELEN 30
>> +#define ARPT_FUNCTION_MAXNAMELEN 29
>>  #define ARPT_TABLE_MAXNAMELEN 32
>> 
>>  #define ARPT_DEV_ADDR_LEN_MAX 16
>> @@ -69,6 +69,8 @@ struct arpt_entry_target
>> 
>>                      /* Used by userspace */
>>                      char name[ARPT_FUNCTION_MAXNAMELEN];
>> +
>> +                    u_int8_t revision;
>
>This structure is not exposed to userspace and I don't find any client
>of it in this code:
>
>include/uapi/linux/netfilter_arp/arp_tables.h:#define arpt_entry_target xt_entry_target
>
>and arp_tables uses xt_entry_target all the time.
revision is used to mark_tg in kernel file : kernel-3.14/net/netfilter/xt_mark.c

static struct xt_target mark_tg_reg __read_mostly = {
        .name           = "MARK",
        .revision       = 2,
        .family         = NFPROTO_UNSPEC,
        .target         = mark_tg,
        .targetsize     = sizeof(struct xt_mark_tginfo2),
        .me             = THIS_MODULE,
};
>
>I guess you can get rid of this dead code. Then, update the cached
>copy and also include x_tables.h definition in the arptables tree.
do you mean ,   in arptable shoudle write like  xtables_target ?

iptables/extensions/libxt_MARK.c
static struct xtables_target mark_tg_reg[] = {
        {  
                .family        = NFPROTO_UNSPEC,
                .name          = "MARK",
                .version       = XTABLES_VERSION,
                .revision      = 0,
                .size          = XT_ALIGN(sizeof(struct xt_mark_target_info)),
                .userspacesize = XT_ALIGN(sizeof(struct xt_mark_target_info)),
                .help          = MARK_help,
                .print         = MARK_print_v0,
                .save          = MARK_save_v0,
                .x6_parse      = MARK_parse_v0,
                .x6_fcheck     = MARK_check,
                .x6_options    = MARK_opts,
        }, 
        {  
                .family        = NFPROTO_IPV4,
                .name          = "MARK",
                .version       = XTABLES_VERSION,
                .revision      = 1,
                .size          = XT_ALIGN(sizeof(struct xt_mark_target_info_v1)),
                .userspacesize = XT_ALIGN(sizeof(struct xt_mark_target_info_v1)),
                .help          = MARK_help,
                .print         = MARK_print_v1,
                .save          = MARK_save_v1,
                .x6_parse      = MARK_parse_v1,
                .x6_fcheck     = MARK_check,
                .x6_options    = MARK_opts,
        },
>
>>              } user;
>>              struct {
>>                      u_int16_t target_size;
>> --
>> 2.1.0
>> 

Re: [PATCH 1/3] Add revision field for xt_entry_target

[Pablo Neira Ayuso]

On Wed, Mar 18, 2015 at 02:47:33AM +0000, Zhang, Chunyu wrote:
> >> diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h
> >> index 0acda66..ccf8cd0 100644
> >> --- a/include/linux/netfilter_arp/arp_tables.h
> >> +++ b/include/linux/netfilter_arp/arp_tables.h
> >> @@ -19,7 +19,7 @@
> >> 
> >>  #include <linux/netfilter_arp.h>
> >> 
> >> -#define ARPT_FUNCTION_MAXNAMELEN 30
> >> +#define ARPT_FUNCTION_MAXNAMELEN 29
> >>  #define ARPT_TABLE_MAXNAMELEN 32
> >> 
> >>  #define ARPT_DEV_ADDR_LEN_MAX 16
> >> @@ -69,6 +69,8 @@ struct arpt_entry_target
> >> 
> >>                      /* Used by userspace */
> >>                      char name[ARPT_FUNCTION_MAXNAMELEN];
> >> +
> >> +                    u_int8_t revision;
> >
> >This structure is not exposed to userspace and I don't find any client
> >of it in this code:
> >
> >include/uapi/linux/netfilter_arp/arp_tables.h:#define arpt_entry_target xt_entry_target
> >
> >and arp_tables uses xt_entry_target all the time.
>
> revision is used to mark_tg in kernel file : kernel-3.14/net/netfilter/xt_mark.c

Forget it, I got confused by the net/netfilter/xt_repldata.h trickery,
sorry. Please, resubmit this patch once you have extended the
userspace arp mark extension to exactly mirror what mark can do in
iptables/ip6tables.

Thanks.

Re: Re: [PATCH 1/3] Add revision field for xt_entry_target

[Zhang, Chunyu]

hi pablo
>From: netfilter-devel-owner
>Date: 2015-03-18
>To: Zhang, Chunyu/章 春宇
>Subject: Re: [PATCH 1/3] Add revision field for xt_entry_target
>
>On Wed, Mar 18, 2015 at 02:47:33AM +0000, Zhang, Chunyu wrote:
>> >> diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h
>> >> index 0acda66..ccf8cd0 100644
>> >> --- a/include/linux/netfilter_arp/arp_tables.h
>> >> +++ b/include/linux/netfilter_arp/arp_tables.h
>> >> @@ -19,7 +19,7 @@
>> >> 
>> >>  #include <linux/netfilter_arp.h>
>> >> 
>> >> -#define ARPT_FUNCTION_MAXNAMELEN 30
>> >> +#define ARPT_FUNCTION_MAXNAMELEN 29
>> >>  #define ARPT_TABLE_MAXNAMELEN 32
>> >> 
>> >>  #define ARPT_DEV_ADDR_LEN_MAX 16
>> >> @@ -69,6 +69,8 @@ struct arpt_entry_target
>> >> 
>> >>                      /* Used by userspace */
>> >>                      char name[ARPT_FUNCTION_MAXNAMELEN];
>> >> +
>> >> +                    u_int8_t revision;
>> >
>> >This structure is not exposed to userspace and I don't find any client
>> >of it in this code:
>> >
>> >include/uapi/linux/netfilter_arp/arp_tables.h:#define arpt_entry_target xt_entry_target
>> >
>> >and arp_tables uses xt_entry_target all the time.
>>
>> revision is used to mark_tg in kernel file : kernel-3.14/net/netfilter/xt_mark.c
>
>Forget it, I got confused by the net/netfilter/xt_repldata.h trickery,
>sorry. Please, resubmit this patch once you have extended the
>userspace arp mark extension to exactly mirror what mark can do in
>iptables/ip6tables.
ok, i will do it for revision
also add --and-mark and --or-mark
maybe too many code to add.
>
>Thanks.
>--
>To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: [PATCH 1/3] Add revision field for xt_entry_target

[Pablo Neira Ayuso]

On Wed, Mar 18, 2015 at 09:59:59AM +0000, Zhang, Chunyu wrote:
> >Please, resubmit this patch once you have extended the
> >userspace arp mark extension to exactly mirror what mark can do in
> >iptables/ip6tables.
>
> ok, i will do it for revision
> also add --and-mark and --or-mark
> maybe too many code to add.

Please, add exactly the same:

"MARK target options:\n"
"  --set-mark value                   Set nfmark value\n"
"  --and-mark value                   Binary AND the nfmark with value\n"
"  --or-mark  value                   Binary OR  the nfmark with value\n");

We'll have arptables-compat over nftables ready soon, and things get
easier for us if we get arptables in sync with iptables.

Thanks.

Re: Re: [PATCH 1/3] Add revision field for xt_entry_target

[Zhang, Chunyu]

>From: Pablo Neira Ayuso
>Date: 2015-03-18
>To: Zhang, Chunyu/章 春宇
>Subject: Re: Re: [PATCH 1/3] Add revision field for xt_entry_target
>
>On Wed, Mar 18, 2015 at 09:59:59AM +0000, Zhang, Chunyu wrote:
>> >Please, resubmit this patch once you have extended the
>> >userspace arp mark extension to exactly mirror what mark can do in
>> >iptables/ip6tables.
>>
>> ok, i will do it for revision
>> also add --and-mark and --or-mark
>> maybe too many code to add.
>
>Please, add exactly the same:
>
>"MARK target options:\n"
>"  --set-mark value                   Set nfmark value\n"
>"  --and-mark value                   Binary AND the nfmark with value\n"
>"  --or-mark  value                   Binary OR  the nfmark with value\n");
ok, got it!

thanks.
>
>We'll have arptables-compat over nftables ready soon, and things get
>easier for us if we get arptables in sync with iptables.
>
>Thanks.

[PATCH 1/3] Add revision field for xt_entry_target

[Gao feng]

This filed is useful if we want to add TARGET which
has revision for arptables rules.

Also make sure xt_entry_target is consistent with
the definition in kernel.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/linux/netfilter_arp/arp_tables.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h
index 0acda66..ccf8cd0 100644
--- a/include/linux/netfilter_arp/arp_tables.h
+++ b/include/linux/netfilter_arp/arp_tables.h
@@ -19,7 +19,7 @@
 
 #include <linux/netfilter_arp.h>
 
-#define ARPT_FUNCTION_MAXNAMELEN 30
+#define ARPT_FUNCTION_MAXNAMELEN 29
 #define ARPT_TABLE_MAXNAMELEN 32
 
 #define ARPT_DEV_ADDR_LEN_MAX 16
@@ -69,6 +69,8 @@ struct arpt_entry_target
 
 			/* Used by userspace */
 			char name[ARPT_FUNCTION_MAXNAMELEN];
+
+			u_int8_t revision;
 		} user;
 		struct {
 			u_int16_t target_size;
-- 
2.1.0

Re: [PATCH 1/3] Add revision field for xt_entry_target

[Pablo Neira Ayuso]

On Fri, Feb 06, 2015 at 03:26:28PM +0800, Gao feng wrote:
> This filed is useful if we want to add TARGET which
> has revision for arptables rules.
> 
> Also make sure xt_entry_target is consistent with
> the definition in kernel.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  include/linux/netfilter_arp/arp_tables.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/netfilter_arp/arp_tables.h b/include/linux/netfilter_arp/arp_tables.h
> index 0acda66..ccf8cd0 100644
> --- a/include/linux/netfilter_arp/arp_tables.h
> +++ b/include/linux/netfilter_arp/arp_tables.h
> @@ -19,7 +19,7 @@
>  
>  #include <linux/netfilter_arp.h>
>  
> -#define ARPT_FUNCTION_MAXNAMELEN 30
> +#define ARPT_FUNCTION_MAXNAMELEN 29
>  #define ARPT_TABLE_MAXNAMELEN 32
>  
>  #define ARPT_DEV_ADDR_LEN_MAX 16
> @@ -69,6 +69,8 @@ struct arpt_entry_target
>  
>  			/* Used by userspace */
>  			char name[ARPT_FUNCTION_MAXNAMELEN];
> +
> +			u_int8_t revision;

This structure is not exposed to userspace and I don't find any client
of it in this code:

include/uapi/linux/netfilter_arp/arp_tables.h:#define arpt_entry_target xt_entry_target

and arp_tables uses xt_entry_target all the time.

I guess you can get rid of this dead code. Then, update the cached
copy and also include x_tables.h definition in the arptables tree.

>  		} user;
>  		struct {
>  			u_int16_t target_size;
> -- 
> 2.1.0
>