[PATCH] xt_connmark: Add bit mapping for bit-shift operation.

[Jack Ma]

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 
Signed-off-by: Jack Ma 
---
 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] xt_connmark: Add bit mapping for bit-shift operation.

[Pablo Neira Ayuso]

On Fri, Apr 06, 2018 at 11:14:12AM +0200, Florian Westphal wrote:
> Jack Ma  wrote:
> > 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 
> 
> I don't recall having seen this patch before.
> 
> That being said, it looks ok to me.
> 
> You might have mentioned that this patch is for 'nf' tree, after
> having merged -next, as this patch changes user-visible behaviour
> (which should be fine in this case as the original change isn't part
>  of 4.16).

Applied to nf.git including the Fixes: tag.
--
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_connmark: Add bit mapping for bit-shift operation.

[Florian Westphal]

Jack Ma  wrote:
> 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 

I don't recall having seen this patch before.

That being said, it looks ok to me.

You might have mentioned that this patch is for 'nf' tree, after
having merged -next, as this patch changes user-visible behaviour
(which should be fine in this case as the original change isn't part
 of 4.16).
--
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] xt_connmark: Add bit mapping for bit-shift operation.

[Jack Ma]

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 
Signed-off-by: Jack Ma 
---
 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