Re: [PATCH v5] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
Hi Pablo, Sure thank you, Will do. Regards, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Change since v2: 1) Fix invalid data pointer caster which would result in loss of the significance. Change since v3: 1) Fix serveral indentation problems. Change since v4: 1) Hide 'shift' operations when it is not set. Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 293 -- include/linux/netfilter/xt_connmark.h | 5 + 2 files changed, 285 insertions(+), 13 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..1a859456 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -32,28 +32,42 @@ struct xt_connmark_target_info { }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | -F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | +F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, +}; + +static const char *const xt_connmark_shift_ops[] = { + "left-shift-mark", + "right-shift-mark" }; static void CONNMARK_help(void) @@ -104,6 +118,36 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8, +.min = 0, .max = 32}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8, +.min = 0, .max = 32}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, +.excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +166,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left s
[PATCH v4] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Change since v2: 1) Fix invalid data pointer caster which would result in loss of the significance. Change since v3: 1) Fix serveral indentation problems. Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 286 -- include/linux/netfilter/xt_connmark.h | 5 + 2 files changed, 279 insertions(+), 12 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..cc9116c9 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -32,30 +32,42 @@ struct xt_connmark_target_info { }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | -F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | +F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; +static const char *const xt_connmark_shift_ops[] = + { "left-shift-mark", "right-shift-mark" }; + static void CONNMARK_help(void) { printf( @@ -104,6 +116,36 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8, +.min = 0, .max = 32}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8, +.min = 0, .max = 32}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, +.excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +164,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark v
[PATCH v2] iptables: Set wait to true by default.
This allow each iptables user to wait for the xtable_lock for a maximum of 1 second by default. It's uncommon to not wait for xtables_lock and exit immediately if the lock is not available. If latency-senstive applications require the wait interval to be re-adjusted, users are still able to set wait_interval back to zero or more precised value to fit in their application. Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- iptables/ip6tables-restore.c | 3 ++- iptables/ip6tables.c | 4 ++-- iptables/iptables-restore.8.in | 6 +++--- iptables/iptables-restore.c| 3 ++- iptables/iptables.8.in | 6 +++--- iptables/iptables.c| 4 ++-- iptables/xtables.c | 4 ++-- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c index 47310f20..cdd634f3 100644 --- a/iptables/ip6tables-restore.c +++ b/iptables/ip6tables-restore.c @@ -26,8 +26,9 @@ #define DEBUGP(x, args...) #endif -static int counters, verbose, noflush, wait; +static int counters, verbose, noflush; +static int wait = 1; static struct timeval wait_interval = { .tv_sec = 1, }; diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c index 49bd006f..693de28a 100644 --- a/iptables/ip6tables.c +++ b/iptables/ip6tables.c @@ -1338,11 +1338,11 @@ int do_command6(int argc, char *argv[], char **table, struct in6_addr *smasks = NULL, *dmasks = NULL; int verbose = 0; - int wait = 0; + int wait = 1; struct timeval wait_interval = { .tv_sec = 1, }; - bool wait_interval_set = false; + bool wait_interval_set = true; const char *chain = NULL; const char *shostnetworkmask = NULL, *dhostnetworkmask = NULL; const char *policy = NULL, *newname = NULL; diff --git a/iptables/iptables-restore.8.in b/iptables/iptables-restore.8.in index f751492d..fb8dc970 100644 --- a/iptables/iptables-restore.8.in +++ b/iptables/iptables-restore.8.in @@ -62,9 +62,9 @@ Print the program version number. Wait for the xtables lock. To prevent multiple instances of the program from running concurrently, an attempt will be made to obtain an exclusive lock at launch. By default, -the program will exit if the lock cannot be obtained. This option will -make the program wait (indefinitely or for optional \fIseconds\fP) until -the exclusive lock can be obtained. +the program will wait for a maximum time of 1 second before exiting. +This option will make the program wait (indefinitely or for optional +\fIseconds\fP) until the exclusive lock can be obtained. .TP \fB\-W\fP, \fB\-\-wait-interval\fP \fImicroseconds\fP Interval to wait per each iteration. diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c index 074552af..3cd168a1 100644 --- a/iptables/iptables-restore.c +++ b/iptables/iptables-restore.c @@ -23,8 +23,9 @@ #define DEBUGP(x, args...) #endif -static int counters, verbose, noflush, wait; +static int counters, verbose, noflush; +static int wait = 1; static struct timeval wait_interval = { .tv_sec = 1, }; diff --git a/iptables/iptables.8.in b/iptables/iptables.8.in index a9c6b252..54fe33df 100644 --- a/iptables/iptables.8.in +++ b/iptables/iptables.8.in @@ -366,9 +366,9 @@ specified multiple times to possibly emit more detailed debug statements. Wait for the xtables lock. To prevent multiple instances of the program from running concurrently, an attempt will be made to obtain an exclusive lock at launch. By default, -the program will exit if the lock cannot be obtained. This option will -make the program wait (indefinitely or for optional \fIseconds\fP) until -the exclusive lock can be obtained. +the program will wait for a maximum time of 1 second before exiting. +This option will make the program wait (indefinitely or for optional +\fIseconds\fP) until the exclusive lock can be obtained. .TP \fB\-W\fP, \fB\-\-wait-interval\fP \fImicroseconds\fP Interval to wait per each iteration. diff --git a/iptables/iptables.c b/iptables/iptables.c index 69d19fec..540d1d60 100644 --- a/iptables/iptables.c +++ b/iptables/iptables.c @@ -1333,9 +1333,9 @@ int do_command4(int argc, char *argv[], char **table, struct timeval wait_interval = { .tv_sec = 1, }; - bool wait_interval_set = false; + bool wait_interval_set = true; int verbose = 0; - int wait = 0; + int wait = 1; const char *chain = NULL; const char *shostnetworkmask = NULL, *dhostnetworkmask = NULL; const char *policy = NULL, *newname = NULL; diff --git a/iptables/xtables.c b/iptables/xtables.c index ac113254..653362fe 100644 --- a/iptables/xtables.c +++ b/iptables/xtables.c @@ -689,10 +689,10 @@ void do_parse(struct nft_handle *h, int argc, char *argv[], { struct xtables_match *m; struct xtables_rule_match *matchp; - bool wait_interval_set =
[PATCH] iptables: Set wait to true by default.
This allow each iptables user to wait for the xtable_lock for a maximum of 1 second by default. It's uncommon to not wait for xtables_lock and exit immediately if the lock is not available. If latency-senstive applications require the wait interval to be re-adjusted, users are still able to set wait_interval back to zero or more precised value to fit in their application. Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- iptables/ip6tables-restore.c | 3 ++- iptables/ip6tables.c | 4 ++-- iptables/iptables-restore.8.in | 6 +++--- iptables/iptables-restore.c| 3 ++- iptables/iptables.8.in | 6 +++--- iptables/iptables.c| 4 ++-- iptables/xtables.c | 4 ++-- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c index 47310f20..cdd634f3 100644 --- a/iptables/ip6tables-restore.c +++ b/iptables/ip6tables-restore.c @@ -26,8 +26,9 @@ #define DEBUGP(x, args...) #endif -static int counters, verbose, noflush, wait; +static int counters, verbose, noflush; +static int wait = 1; static struct timeval wait_interval = { .tv_sec = 1, }; diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c index 49bd006f..693de28a 100644 --- a/iptables/ip6tables.c +++ b/iptables/ip6tables.c @@ -1338,11 +1338,11 @@ int do_command6(int argc, char *argv[], char **table, struct in6_addr *smasks = NULL, *dmasks = NULL; int verbose = 0; - int wait = 0; + int wait = 1; struct timeval wait_interval = { .tv_sec = 1, }; - bool wait_interval_set = false; + bool wait_interval_set = true; const char *chain = NULL; const char *shostnetworkmask = NULL, *dhostnetworkmask = NULL; const char *policy = NULL, *newname = NULL; diff --git a/iptables/iptables-restore.8.in b/iptables/iptables-restore.8.in index f751492d..fb8dc970 100644 --- a/iptables/iptables-restore.8.in +++ b/iptables/iptables-restore.8.in @@ -62,9 +62,9 @@ Print the program version number. Wait for the xtables lock. To prevent multiple instances of the program from running concurrently, an attempt will be made to obtain an exclusive lock at launch. By default, -the program will exit if the lock cannot be obtained. This option will -make the program wait (indefinitely or for optional \fIseconds\fP) until -the exclusive lock can be obtained. +the program will wait for a maximum time of 1 second before exiting. +This option will make the program wait (indefinitely or for optional +\fIseconds\fP) until the exclusive lock can be obtained. .TP \fB\-W\fP, \fB\-\-wait-interval\fP \fImicroseconds\fP Interval to wait per each iteration. diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c index 074552af..b1fa152e 100644 --- a/iptables/iptables-restore.c +++ b/iptables/iptables-restore.c @@ -23,8 +23,9 @@ #define DEBUGP(x, args...) #endif -static int counters, verbose, noflush, wait; +static int counters, verbose, noflush; +static wait = 1; static struct timeval wait_interval = { .tv_sec = 1, }; diff --git a/iptables/iptables.8.in b/iptables/iptables.8.in index a9c6b252..54fe33df 100644 --- a/iptables/iptables.8.in +++ b/iptables/iptables.8.in @@ -366,9 +366,9 @@ specified multiple times to possibly emit more detailed debug statements. Wait for the xtables lock. To prevent multiple instances of the program from running concurrently, an attempt will be made to obtain an exclusive lock at launch. By default, -the program will exit if the lock cannot be obtained. This option will -make the program wait (indefinitely or for optional \fIseconds\fP) until -the exclusive lock can be obtained. +the program will wait for a maximum time of 1 second before exiting. +This option will make the program wait (indefinitely or for optional +\fIseconds\fP) until the exclusive lock can be obtained. .TP \fB\-W\fP, \fB\-\-wait-interval\fP \fImicroseconds\fP Interval to wait per each iteration. diff --git a/iptables/iptables.c b/iptables/iptables.c index 69d19fec..540d1d60 100644 --- a/iptables/iptables.c +++ b/iptables/iptables.c @@ -1333,9 +1333,9 @@ int do_command4(int argc, char *argv[], char **table, struct timeval wait_interval = { .tv_sec = 1, }; - bool wait_interval_set = false; + bool wait_interval_set = true; int verbose = 0; - int wait = 0; + int wait = 1; const char *chain = NULL; const char *shostnetworkmask = NULL, *dhostnetworkmask = NULL; const char *policy = NULL, *newname = NULL; diff --git a/iptables/xtables.c b/iptables/xtables.c index ac113254..653362fe 100644 --- a/iptables/xtables.c +++ b/iptables/xtables.c @@ -689,10 +689,10 @@ void do_parse(struct nft_handle *h, int argc, char *argv[], { struct xtables_match *m; struct xtables_rule_match *matchp; - bool wait_interval_set = false; +
Re: [PATCH nf] netfilter: xt_connmark: do not cast xt_connmark_tginfo1 to xt_connmark_tginfo2
Hi Pablo, Thanks, looks good! Regards, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
Hi Florian, This patch has not been applied yet. This is actually a older version of the patch thats reviewed :P, unfortunately I refactored it bit and caused this regression. Do we still need the section "changes since v2" ? To me, this should be the first patch that starts supporting v2. I might've misunderstood your intention could you please explain a bit more? Thanks, Jack -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xt_connmark: Fix invalid tginfo* casting.
Sorry, please ignore v1 patch. version two is the clean patch to be applied.. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] xt_connmark: Fix invalid tginfo* casting.
This bug was found during testing pre-existing iptables options with newly added v2 APIs. Casting from (const struct xt_connmark_tginfo2 *) to (const struct xt_connmark_tginfo1 *) results in the significance to be lost. Subsequentially, the 'info->mode' is reset to zero, when multiple options are parsed in. Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- net/netfilter/xt_connmark.c | 25 ++--- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c index 710bc2bfe020..4350a6877077 100644 --- a/net/netfilter/xt_connmark.c +++ b/net/netfilter/xt_connmark.c @@ -37,21 +37,22 @@ MODULE_ALIAS("ip6t_connmark"); static unsigned int connmark_tg_shift(struct sk_buff *skb, - const struct xt_connmark_tginfo1 *info, + u8 mode, u32 ctmark, + u32 ctmask, u32 nfmask, u8 shift_bits, u8 shift_dir) { enum ip_conntrack_info ctinfo; struct nf_conn *ct; - u_int32_t newmark; - u_int32_t new_targetmark; + u_int32_t newmark = 0; + u_int32_t new_targetmark = 0; ct = nf_ct_get(skb, ); if (ct == NULL) return XT_CONTINUE; - switch (info->mode) { + switch (mode) { case XT_CONNMARK_SET: - newmark = (ct->mark & ~info->ctmask) ^ info->ctmark; + newmark = (ct->mark & ~ctmask) ^ ctmark; if (shift_dir == D_SHIFT_RIGHT) newmark >>= shift_bits; else @@ -62,12 +63,12 @@ connmark_tg_shift(struct sk_buff *skb, } break; case XT_CONNMARK_SAVE: - new_targetmark = (skb->mark & info->nfmask); + new_targetmark = (skb->mark & nfmask); if (shift_dir == D_SHIFT_RIGHT) new_targetmark >>= shift_bits; else new_targetmark <<= shift_bits; - newmark = (ct->mark & ~info->ctmask) ^ + newmark = (ct->mark & ~ctmask) ^ new_targetmark; if (ct->mark != newmark) { ct->mark = newmark; @@ -75,12 +76,12 @@ connmark_tg_shift(struct sk_buff *skb, } break; case XT_CONNMARK_RESTORE: - new_targetmark = (ct->mark & info->ctmask); + new_targetmark = (ct->mark & ctmask); if (shift_dir == D_SHIFT_RIGHT) new_targetmark >>= shift_bits; else new_targetmark <<= shift_bits; - newmark = (skb->mark & ~info->nfmask) ^ + newmark = (skb->mark & ~nfmask) ^ new_targetmark; skb->mark = newmark; break; @@ -93,7 +94,8 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) { const struct xt_connmark_tginfo1 *info = par->targinfo; - return connmark_tg_shift(skb, info, 0, 0); + return connmark_tg_shift(skb, info->mode, info->ctmark, +info->ctmask, info->nfmask, 0, 0); } static unsigned int @@ -101,7 +103,8 @@ connmark_tg_v2(struct sk_buff *skb, const struct xt_action_param *par) { const struct xt_connmark_tginfo2 *info = par->targinfo; - return connmark_tg_shift(skb, (const struct xt_connmark_tginfo1 *)info, + return connmark_tg_shift(skb, info->mode, info->ctmark, +info->ctmask, info->nfmask, info->shift_bits, info->shift_dir); } -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
The same fix from: https://patchwork.ozlabs.org/patch/898351/ applied for libxt_CONNMARK.c-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 286 -- include/linux/netfilter/xt_connmark.h | 5 + 2 files changed, 279 insertions(+), 12 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..499c0b61 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -32,30 +32,42 @@ struct xt_connmark_target_info { }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | -F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | +F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; +static const char *const xt_connmark_shift_ops[] = + { "left-shift-mark", "right-shift-mark" }; + static void CONNMARK_help(void) { printf( @@ -104,6 +116,36 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8, +.min = 0, .max = 32}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8, +.min = 0, .max = 32}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, +.excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +164,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark value Right shift the ctmark with bits\n" +); +} + static void connmark_tg_init(struct xt_entry_target *target) { struct xt_connmark_tginfo1 *info = (
[PATCH] xt_connmark: Add bit mapping for bit-shift operation.
With the additiona of bit-shift operations, we are able to shift ct/skbmark based on user requirements. However, this change might also cause the most left/right hand- side mark to be accidentially lost during shift operations. This patch adds the ability to 'grep' ceratin bits based on ctmask or nfmask out of the original mark. Then apply shift operations to achieve a new mapping between ctmark and skb->mark. For example. If someone would like save the fourth F bits of ctmark 0xFFF(F)000F into the seventh hexadecimal (0) skb->mark 0xABC000(0)E. new_targetmark = (ctmark & ctmask) >> 12; (new) skb->mark = (skb->mark &~nfmask) ^ new_targetmark; This will preserve the other bits that are not related to this operation. Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- net/netfilter/xt_connmark.c | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c index 773da82190dc..4247f437dcae 100644 --- a/net/netfilter/xt_connmark.c +++ b/net/netfilter/xt_connmark.c @@ -37,20 +37,22 @@ MODULE_ALIAS("ip6t_connmark"); static unsigned int connmark_tg_shift(struct sk_buff *skb, - const struct xt_connmark_tginfo1 *info, + u8 mode, u32 ctmark, + u32 ctmask, u32 nfmask, u8 shift_bits, u8 shift_dir) { enum ip_conntrack_info ctinfo; struct nf_conn *ct; u_int32_t newmark; + u_int32_t new_targetmark; ct = nf_ct_get(skb, ); if (ct == NULL) return XT_CONTINUE; - switch (info->mode) { + switch (mode) { case XT_CONNMARK_SET: - newmark = (ct->mark & ~info->ctmask) ^ info->ctmark; + newmark = (ct->mark & ~ctmask) ^ ctmark; if (shift_dir == D_SHIFT_RIGHT) newmark >>= shift_bits; else @@ -61,24 +63,26 @@ connmark_tg_shift(struct sk_buff *skb, } break; case XT_CONNMARK_SAVE: - newmark = (ct->mark & ~info->ctmask) ^ - (skb->mark & info->nfmask); + new_targetmark = (skb->mark & nfmask); if (shift_dir == D_SHIFT_RIGHT) - newmark >>= shift_bits; + new_targetmark >>= shift_bits; else - newmark <<= shift_bits; + new_targetmark <<= shift_bits; + newmark = (ct->mark & ~ctmask) ^ + new_targetmark; if (ct->mark != newmark) { ct->mark = newmark; nf_conntrack_event_cache(IPCT_MARK, ct); } break; case XT_CONNMARK_RESTORE: - newmark = (skb->mark & ~info->nfmask) ^ - (ct->mark & info->ctmask); + new_targetmark = (ct->mark & ctmask); if (shift_dir == D_SHIFT_RIGHT) - newmark >>= shift_bits; + new_targetmark >>= shift_bits; else - newmark <<= shift_bits; + new_targetmark <<= shift_bits; + newmark = (skb->mark & ~nfmask) ^ + new_targetmark; skb->mark = newmark; break; } @@ -90,7 +94,8 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) { const struct xt_connmark_tginfo1 *info = par->targinfo; - return connmark_tg_shift(skb, info, 0, 0); + return connmark_tg_shift(skb, info->mode, info->ctmark, +info->ctmask, info->nfmask, 0, 0); } static unsigned int @@ -98,7 +103,8 @@ connmark_tg_v2(struct sk_buff *skb, const struct xt_action_param *par) { const struct xt_connmark_tginfo2 *info = par->targinfo; - return connmark_tg_shift(skb, (const struct xt_connmark_tginfo1 *)info, + return connmark_tg_shift(skb, info->mode, info->ctmark, +info->ctmask, info->nfmask, info->shift_bits, info->shift_dir); } -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
Hi Florian & Pablo, The pointer casting from (struct xt_connmark_tginfo2 *) to (const struct xt_connmark_tginfo1 *) cause the significance to be lost... Sorry this is a silly mistake I made.. Already got patches addressing the issue. Plan to send out for review next week.. Regards, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 161 +++--- include/linux/netfilter/xt_connmark.h | 5 ++ 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..3551b594 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -28,34 +28,48 @@ struct xt_connmark_target_info { unsigned long mark; unsigned long mask; + uint8_t shift_dir; + uint8_t shift_bits; uint8_t mode; }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | -F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | +F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; +static const char *const xt_connmark_shift_ops[] = + { "left-shift", "right-shift" }; + static void CONNMARK_help(void) { printf( @@ -104,6 +118,34 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, +.excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +164,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark value Right shift the ctmark with bits\n" +); +} + static void connmark_tg_init(struct
[PATCH v2] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 161 +++--- include/linux/netfilter/xt_connmark.h | 5 ++ 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..3551b594 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -28,34 +28,48 @@ struct xt_connmark_target_info { unsigned long mark; unsigned long mask; + uint8_t shift_dir; + uint8_t shift_bits; uint8_t mode; }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | -F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | +F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; +static const char *const xt_connmark_shift_ops[] = + { "left-shift", "right-shift" }; + static void CONNMARK_help(void) { printf( @@ -104,6 +118,34 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, +.excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +164,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark value Right shift the ctmark with bits\n" +); +} + static void connmark_tg_init(struct
[PATCH] xt_connmark: Add bit mapping for bit-shift operation.
With the additiona of bit-shift operations, we are able to shift ct/skbmark based on user requirements. However, this change might also cause the most left/right hand- side mark to be accidentially lost during shift operations. This patch adds the ability to 'grep' ceratin bits based on ctmask or nfmask out of the original mark. Then apply shift operations to achieve a new mapping between ctmark and skb->mark. For example. If someone would like save the fourth F bits of ctmark 0xFFF(F)000F into the seventh hexadecimal (0) skb->mark 0xABC000(0)E. new_targetmark = (ctmark & ctmask) >> 12; (new) skb->mark = (skb->mark &~nfmask) ^ new_targetmark; This will preserve the other bits that are not related to this operation. Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- net/netfilter/xt_connmark.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c index 773da82190dc..710bc2bfe020 100644 --- a/net/netfilter/xt_connmark.c +++ b/net/netfilter/xt_connmark.c @@ -43,6 +43,7 @@ connmark_tg_shift(struct sk_buff *skb, enum ip_conntrack_info ctinfo; struct nf_conn *ct; u_int32_t newmark; + u_int32_t new_targetmark; ct = nf_ct_get(skb, ); if (ct == NULL) @@ -61,24 +62,26 @@ connmark_tg_shift(struct sk_buff *skb, } break; case XT_CONNMARK_SAVE: - newmark = (ct->mark & ~info->ctmask) ^ - (skb->mark & info->nfmask); + new_targetmark = (skb->mark & info->nfmask); if (shift_dir == D_SHIFT_RIGHT) - newmark >>= shift_bits; + new_targetmark >>= shift_bits; else - newmark <<= shift_bits; + new_targetmark <<= shift_bits; + newmark = (ct->mark & ~info->ctmask) ^ + new_targetmark; if (ct->mark != newmark) { ct->mark = newmark; nf_conntrack_event_cache(IPCT_MARK, ct); } break; case XT_CONNMARK_RESTORE: - newmark = (skb->mark & ~info->nfmask) ^ - (ct->mark & info->ctmask); + new_targetmark = (ct->mark & info->ctmask); if (shift_dir == D_SHIFT_RIGHT) - newmark >>= shift_bits; + new_targetmark >>= shift_bits; else - newmark <<= shift_bits; + new_targetmark <<= shift_bits; + newmark = (skb->mark & ~info->nfmask) ^ + new_targetmark; skb->mark = newmark; break; } -- 2.13.0 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Proposal: Add config option to set xtable_lock wait = true.
Hi Florian & Pablo, I noticed that lots iptables users are likely to miss the '-w' option while implementing multi-process program. Due to the fact that the iptables and ip6tables do not wait for the xtable_lock, people can easily mis-configure their iptables command because of concurrency issues. I'd like to propose a global config option to set the default wait interval and allow iptables to always wait for the lock. ie. " iptables --always-wait (ms) " if no value is specified, then use the default 1 second. I found it hard to see any users who may wish to run iptables command without lock. Does this proposal sound sane-ish ? Regards, Jack -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xt_conntrack: Support bit-shifting for CONNMARK & MARK targets.
Hi Pablo, Sorry for bothering you again... Is there any feedback for this patch: https://patchwork.ozlabs.org/patch/887481/ I am planning to do a iptables update recently, would be nice if this patch is applied.. Also, I haven't heard any feedback for a while, seems like I might've missed some review comments. Any update? Thanks, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark
This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 161 +++--- include/linux/netfilter/xt_connmark.h | 5 ++ 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..331fdc78 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -28,34 +28,48 @@ struct xt_connmark_target_info { unsigned long mark; unsigned long mask; + uint8_t shift_dir; + uint8_t shift_bits; uint8_t mode; }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | -F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | +F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; +static const char *const xt_connmark_shift_ops[] = + { "left-shift", "right-shift" }; + static void CONNMARK_help(void) { printf( @@ -104,6 +118,34 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, +.excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, +.excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, +.excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, +.excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, +.excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +164,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark value Right shift the ctmark with bits\n" +); +} + static void connmark_tg_init(struct
[PATCH] xt_conntrack: Support bit-shifting for CONNMARK & MARK targets.
This patch introduces a new feature that allows bitshifting (left and right) operations to co-operate with existing iptables options. Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- include/uapi/linux/netfilter/xt_connmark.h | 10 net/netfilter/xt_connmark.c| 77 +++--- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/include/uapi/linux/netfilter/xt_connmark.h b/include/uapi/linux/netfilter/xt_connmark.h index 408a9654f05c..1aa5c955ee1e 100644 --- a/include/uapi/linux/netfilter/xt_connmark.h +++ b/include/uapi/linux/netfilter/xt_connmark.h @@ -19,11 +19,21 @@ enum { XT_CONNMARK_RESTORE }; +enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + struct xt_connmark_tginfo1 { __u32 ctmark, ctmask, nfmask; __u8 mode; }; +struct xt_connmark_tginfo2 { + __u32 ctmark, ctmask, nfmask; + __u8 shift_dir, shift_bits, mode; +}; + struct xt_connmark_mtinfo1 { __u32 mark, mask; __u8 invert; diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c index 809639ce6f5a..773da82190dc 100644 --- a/net/netfilter/xt_connmark.c +++ b/net/netfilter/xt_connmark.c @@ -36,9 +36,10 @@ MODULE_ALIAS("ipt_connmark"); MODULE_ALIAS("ip6t_connmark"); static unsigned int -connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) +connmark_tg_shift(struct sk_buff *skb, + const struct xt_connmark_tginfo1 *info, + u8 shift_bits, u8 shift_dir) { - const struct xt_connmark_tginfo1 *info = par->targinfo; enum ip_conntrack_info ctinfo; struct nf_conn *ct; u_int32_t newmark; @@ -50,6 +51,10 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) switch (info->mode) { case XT_CONNMARK_SET: newmark = (ct->mark & ~info->ctmask) ^ info->ctmark; + if (shift_dir == D_SHIFT_RIGHT) + newmark >>= shift_bits; + else + newmark <<= shift_bits; if (ct->mark != newmark) { ct->mark = newmark; nf_conntrack_event_cache(IPCT_MARK, ct); @@ -57,7 +62,11 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) break; case XT_CONNMARK_SAVE: newmark = (ct->mark & ~info->ctmask) ^ - (skb->mark & info->nfmask); + (skb->mark & info->nfmask); + if (shift_dir == D_SHIFT_RIGHT) + newmark >>= shift_bits; + else + newmark <<= shift_bits; if (ct->mark != newmark) { ct->mark = newmark; nf_conntrack_event_cache(IPCT_MARK, ct); @@ -65,14 +74,34 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) break; case XT_CONNMARK_RESTORE: newmark = (skb->mark & ~info->nfmask) ^ - (ct->mark & info->ctmask); + (ct->mark & info->ctmask); + if (shift_dir == D_SHIFT_RIGHT) + newmark >>= shift_bits; + else + newmark <<= shift_bits; skb->mark = newmark; break; } - return XT_CONTINUE; } +static unsigned int +connmark_tg(struct sk_buff *skb, const struct xt_action_param *par) +{ + const struct xt_connmark_tginfo1 *info = par->targinfo; + + return connmark_tg_shift(skb, info, 0, 0); +} + +static unsigned int +connmark_tg_v2(struct sk_buff *skb, const struct xt_action_param *par) +{ + const struct xt_connmark_tginfo2 *info = par->targinfo; + + return connmark_tg_shift(skb, (const struct xt_connmark_tginfo1 *)info, +info->shift_bits, info->shift_dir); +} + static int connmark_tg_check(const struct xt_tgchk_param *par) { int ret; @@ -119,15 +148,27 @@ static void connmark_mt_destroy(const struct xt_mtdtor_param *par) nf_ct_netns_put(par->net, par->family); } -static struct xt_target connmark_tg_reg __read_mostly = { - .name = "CONNMARK", - .revision = 1, - .family = NFPROTO_UNSPEC, - .checkentry = connmark_tg_check, - .target = connmark_tg, - .targetsize = sizeof(struct xt_connmark_tginfo1), - .destroy= connmark_tg_destroy, - .me = THIS_MODULE, +static struct xt_target connmark_tg_reg[] __read_mostly = { + { + .name = "CONNMARK", + .revision = 1, + .
Re: [PATCH] support bit shifting operations
Hi Florian, Are these patches likely to be reviewed recently? Also, any recommended maintainer for delivery :P? Thanks, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] support bit shifting operations
Attached two patches. From ae4b151a8e8f86758aa0a7ac79a6f890c068d73e Mon Sep 17 00:00:00 2001 From: Jack Ma <jack...@alliedtelesis.co.nz> Date: Mon, 12 Feb 2018 13:41:29 +1300 Subject: [PATCH] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark This patch adds a new feature to iptables that allow bitshifting for --restore,set and save-mark operations. This allows existing logic operators (and, or and xor) and mask to co-operate with new bitshift operations. The intention is to provide uses with more fexible uses of skb->mark and ct->mark. For example, users can save extra bits in skb->mark: skb->mark = ct->mark << 8; Reviewed-by: Florian Westphal <f...@strlen.de> Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> --- extensions/libxt_CONNMARK.c | 161 +++--- include/linux/netfilter/xt_connmark.h | 5 ++ 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index 94984cdc..331fdc78 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -28,34 +28,48 @@ struct xt_connmark_target_info { unsigned long mark; unsigned long mask; + uint8_t shift_dir; + uint8_t shift_bits; uint8_t mode; }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | - F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | + F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; +static const char *const xt_connmark_shift_ops[] = + { "left-shift", "right-shift" }; + static void CONNMARK_help(void) { printf( @@ -104,6 +118,34 @@ static const struct xt_option_entry connmark_tg_opts[] = { }; #undef s +#define s struct xt_connmark_tginfo2 +static const struct xt_option_entry connmark_tg_opts_v2[] = { + {.name = "set-xmark", .id = O_SET_XMARK, .type = XTTYPE_MARKMASK32, + .excl = F_OP_ANY}, + {.name = "set-mark", .id = O_SET_MARK, .type = XTTYPE_MARKMASK32, + .excl = F_OP_ANY}, + {.name = "and-mark", .id = O_AND_MARK, .type = XTTYPE_UINT32, + .excl = F_OP_ANY}, + {.name = "or-mark", .id = O_OR_MARK, .type = XTTYPE_UINT32, + .excl = F_OP_ANY}, + {.name = "xor-mark", .id = O_XOR_MARK, .type = XTTYPE_UINT32, + .excl = F_OP_ANY}, + {.name = "save-mark", .id = O_SAVE_MARK, .type = XTTYPE_NONE, + .excl = F_OP_ANY}, + {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, + .excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, + .excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, + {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, + .excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, nfmask)}, + {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32, + .excl = F_CTMASK | F_NFMASK}, + XTOPT_TABLEEND, +}; +#undef s + static void connmark_tg_help(void) { printf( @@ -122,6 +164,15 @@ static void connmark_tg_help(void) ); } +static void connmark_tg_help_v2(void) +{ + connmark_tg_help(); + printf( +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark value Right shift the ctmark with bits\n" +); +} + static void connmark_tg_init(struct xt_entry_target *target) { struct xt_connmark_tginfo1 *info = (void *)target->data; @@ -134,6 +185,18 @@ static void connmark_tg_init(struct xt_entry_target *target)
[PATCH] support bit shifting operations
Hi Florian, I think the codes now are much more intuitive after addressing review comments. Both patches should be fairly straight-forward :P I am posting it on the mailing list now. Thanks, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: shift by n bits while performing '--restore-mark'
Hi Florian, Do these patches looks acceptable for the mainline stream? Any code comments will be appreciated ! Regards, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: shift by n bits while performing '--restore-mark'
Hi Florian, I attached two 'draft' patches in this email :) Thanks, JackFrom 6d811e63c9c777ed4287bc4547134c99e939b49d Mon Sep 17 00:00:00 2001 From: Jack Ma <jack...@alliedtelesis.co.nz> Date: Mon, 12 Feb 2018 13:41:29 +1300 Subject: [PATCH] libxt_CONNMARK: Support bit-shifting for --restore,set and save-mark Added bit-shifting operations for --restore & set & save-mark. Signed-off-by: Jack Ma <jack...@alliedtelesis.co.nz> Signed-off-by: Florian Westphal <f...@strlen.de> --- extensions/libxt_CONNMARK.c | 176 ++ include/linux/netfilter/xt_connmark.h | 2 +- 2 files changed, 139 insertions(+), 39 deletions(-) diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c index f60be583..dbd02351 100644 --- a/extensions/libxt_CONNMARK.c +++ b/extensions/libxt_CONNMARK.c @@ -28,32 +28,43 @@ struct xt_connmark_target_info { unsigned long mark; unsigned long mask; + uint8_t shift_dir; + uint8_t shift_bits; uint8_t mode; }; enum { + D_SHIFT_LEFT = 0, + D_SHIFT_RIGHT, +}; + +enum { O_SET_MARK = 0, O_SAVE_MARK, O_RESTORE_MARK, O_AND_MARK, O_OR_MARK, O_XOR_MARK, + O_LEFT_SHIFT_MARK, + O_RIGHT_SHIFT_MARK, O_SET_XMARK, O_CTMASK, O_NFMASK, O_MASK, - F_SET_MARK = 1 << O_SET_MARK, - F_SAVE_MARK= 1 << O_SAVE_MARK, - F_RESTORE_MARK = 1 << O_RESTORE_MARK, - F_AND_MARK = 1 << O_AND_MARK, - F_OR_MARK = 1 << O_OR_MARK, - F_XOR_MARK = 1 << O_XOR_MARK, - F_SET_XMARK= 1 << O_SET_XMARK, - F_CTMASK = 1 << O_CTMASK, - F_NFMASK = 1 << O_NFMASK, - F_MASK = 1 << O_MASK, - F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | - F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, + F_SET_MARK = 1 << O_SET_MARK, + F_SAVE_MARK= 1 << O_SAVE_MARK, + F_RESTORE_MARK = 1 << O_RESTORE_MARK, + F_AND_MARK = 1 << O_AND_MARK, + F_OR_MARK = 1 << O_OR_MARK, + F_XOR_MARK = 1 << O_XOR_MARK, + F_LEFT_SHIFT_MARK = 1 << O_LEFT_SHIFT_MARK, + F_RIGHT_SHIFT_MARK = 1 << O_RIGHT_SHIFT_MARK, + F_SET_XMARK= 1 << O_SET_XMARK, + F_CTMASK = 1 << O_CTMASK, + F_NFMASK = 1 << O_NFMASK, + F_MASK = 1 << O_MASK, + F_OP_ANY = F_SET_MARK | F_SAVE_MARK | F_RESTORE_MARK | + F_AND_MARK | F_OR_MARK | F_XOR_MARK | F_SET_XMARK, }; static void CONNMARK_help(void) @@ -74,6 +85,8 @@ static const struct xt_option_entry CONNMARK_opts[] = { {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, .excl = F_OP_ANY}, {.name = "mask", .id = O_MASK, .type = XTTYPE_UINT32}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8}, XTOPT_TABLEEND, }; #undef s @@ -94,6 +107,8 @@ static const struct xt_option_entry connmark_tg_opts[] = { .excl = F_OP_ANY}, {.name = "restore-mark", .id = O_RESTORE_MARK, .type = XTTYPE_NONE, .excl = F_OP_ANY}, + {.name = "left-shift-mark", .id = O_LEFT_SHIFT_MARK, .type = XTTYPE_UINT8}, + {.name = "right-shift-mark", .id = O_RIGHT_SHIFT_MARK, .type = XTTYPE_UINT8}, {.name = "ctmask", .id = O_CTMASK, .type = XTTYPE_UINT32, .excl = F_MASK, .flags = XTOPT_PUT, XTOPT_POINTER(s, ctmask)}, {.name = "nfmask", .id = O_NFMASK, .type = XTTYPE_UINT32, @@ -119,6 +134,8 @@ static void connmark_tg_help(void) " --and-mark value Binary AND the ctmark with bits\n" " --or-mark value Binary OR the ctmark with bits\n" " --xor-mark value Binary XOR the ctmark with bits\n" +" --left-shift-mark value Left shift the ctmark with bits\n" +" --right-shift-mark value Right shift the ctmark with bits\n" ); } @@ -154,6 +171,16 @@ static void CONNMARK_parse(struct xt_option_call *cb) case O_MASK: markinfo->mask = cb->val.u32; break; + case O_LEFT_SHIFT_MARK: + markinfo->mode = XT_CONNMARK_RESTORE; + markinfo->shift_dir = D_SHIFT_LEFT; + markinfo->shift_bits = cb->val.u8; + break; + case O_RIGHT_SHIFT_MARK: + markinfo->mode = XT_CONNMARK_RESTORE; + markinfo->shift_dir = D_SHIFT_RIGHT; + markinfo->shift_bits = cb->val.u8; + break; } } @@ -197,6 +224,14 @@ static void connmark_tg_parse(struct xt_option_call *cb) case O_MASK: info->nfmask = info->ctmask = cb->val.u32; break; + case O_LEFT_SHIFT_MARK: + info->shift_dir = D_SHIFT_LEFT; + info->shift_bits = cb->val.u8; + break; + case O_RIGHT_SHIFT_MARK: + info->shift_dir = D_SHIFT_RIGHT; + info->shift_bits = cb->val.u8; + break; } } @@ -253,36 +288,101 @@ connmark_tg_print(const voi
shift by n bits while performing '--restore-mark'
Hi Florian, Our current condition is: 1) only 0xfff0 (three F available in skb->mark), but 0xf000 (five F available in ct->mark) We wish to copy either 0xfff0 or 0x00fff000 from ct->mark into skb->mark, What about '-j CONNMARK --restore-mark --mask 0xf000 << 8 ( left shift 2 F)' This will result in skb->mark = ct->mark << 8 if ct->mark = 0xabcde000, now skb->mark is changed to: skb->mark = 0xcde0. Does this make sense :) ? Regards, Jack From: Florian Westphal <f...@strlen.de> Sent: Thursday, January 25, 2018 7:22 PM To: Jack Ma Subject: Re: conntrack enhancement Jack Ma <jack...@alliedtelesis.co.nz> wrote: > Hi Florian, > > Any comments? Please let me know if anything is unclear to you. It would be nice if you could show a pseudo-ruleset that uses this proposed feature, save and restore rule should be enough. Just so I can see why existing mask support isn't sufficient for your use case. Thanks, Florian -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: conntrack enhancement
Hi Florian, Lets start with iptables setting iptables -t mangle Chain EXAMPLE_MAIN pkts bytes target prot opt in out source destination 3709K 204M MARK all -- anyany anywhere anywhere MARK and 0xf 37 6952 CONNMARK all -- anyany anywhere anywhere CONNMARK restore mask 0xfff0 0 0 MARK udp -- anyany anywhere anywhere match and MARK xset 0xabc0/0xfff0 5 308 CONNMARK all -- anyany anywhere anywhere CONNMARK save mask 0xfff0 For the first packet of 'a' flow we mark the packet with fwmark = ct->mark. ip rule then direct traffic using such fwmark. 250:from all fwmark 0xabc lookup TABLE However, we also implemented firewall feature, packet inspection feature etc to use fwmark at the same time. which makes such 32 bit far more occupied than ct->mark (32).. Hopefully this can help! Please let me know if this confuses you.. Thanks, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: conntrack enhancement
(Re-send this again including CC) Hi Florian, I suspect this is for -j CONNMARK --restore-mark / --save-mark ? >> Yes, current thought is to shift bits when mark is restored. Something like skb->mark = ct->mark >> $lshift; ? >> Yes, we shift ct marks. I don't really understand how this is supposed to work. Could you elaborate a bit? >> It's common to run out of fwmark, skb->mark first. This suggestion basically >> attempts to 'grab' certain bits from ct->mark, so we can restore the desired bits into skb->mark. Once skb is marked correctly, we can easily achieve more versatile routing per skb. However, this solution is not going to be scalable in the future. Currently, we still have enough bits in ct->mark left, but way less bits left in skb->mark (which have been occupied by other features thats connectionless)... Thanks, Jack-- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
conntrack enhancement
Hi there, I am planing to add new user option to allow connmark to be shifted to enable more advanced routing options. Currently, it might be something like: Conntrack parameters and options: -sl, --shift-left bits shift mark by n bit to the left -sr, --shift-right bitsshift mark by n bit to the right. We run out of nfmark (skb->mark) in our systems due to increasing number of routes we are supporting. One common user-case: Using connmark to direct traffic via ip rule onto different route tables. But the the first packet of this flow needs to be per-inspected by the IP-tables first to be marked with an ID (Route number ID in our case.). If we can have "SHIFT" operation working in conntrack, we would be able to support much more numbers of 'ID'. I wounder if this "SHIFT" idea can be considered to be accepted by upstream ? Thanks, Jack -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
subscribe netfilter-devel -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
subscribe netdev -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html