netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] skbedit: allow the user to specify bitmask for mark
@ 2014-07-21  8:35 Antonio Quartulli
  2014-07-21 10:43 ` Jamal Hadi Salim
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Antonio Quartulli @ 2014-07-21  8:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, Antonio Quartulli, Jamal Hadi Salim

From: Antonio Quartulli <antonio@open-mesh.com>

The user may want to use only some bits of the skb mark in
his skbedit rules because the remaining part might be used by
something else.

Introduce the "mask" parameter to the skbedit actor in order
to implement such functionality.

When the mask is specified, only those bits selected by the
latter are altered really changed by the actor, while the
rest is left untouched.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 include/net/tc_act/tc_skbedit.h        |  1 +
 include/uapi/linux/tc_act/tc_skbedit.h |  2 ++
 net/sched/act_skbedit.c                | 20 +++++++++++++++++---
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/net/tc_act/tc_skbedit.h b/include/net/tc_act/tc_skbedit.h
index 0df9a0d..a385c68 100644
--- a/include/net/tc_act/tc_skbedit.h
+++ b/include/net/tc_act/tc_skbedit.h
@@ -26,6 +26,7 @@ struct tcf_skbedit {
 	u32			flags;
 	u32     		priority;
 	u32     		mark;
+	u32			mask;
 	u16			queue_mapping;
 	/* XXX: 16-bit pad here? */
 };
diff --git a/include/uapi/linux/tc_act/tc_skbedit.h b/include/uapi/linux/tc_act/tc_skbedit.h
index 7a2e910..c7c8c5f 100644
--- a/include/uapi/linux/tc_act/tc_skbedit.h
+++ b/include/uapi/linux/tc_act/tc_skbedit.h
@@ -27,6 +27,7 @@
 #define SKBEDIT_F_PRIORITY		0x1
 #define SKBEDIT_F_QUEUE_MAPPING		0x2
 #define SKBEDIT_F_MARK			0x4
+#define SKBEDIT_F_MASK			0x8
 
 struct tc_skbedit {
 	tc_gen;
@@ -39,6 +40,7 @@ enum {
 	TCA_SKBEDIT_PRIORITY,
 	TCA_SKBEDIT_QUEUE_MAPPING,
 	TCA_SKBEDIT_MARK,
+	TCA_SKBEDIT_MASK,
 	__TCA_SKBEDIT_MAX
 };
 #define TCA_SKBEDIT_MAX (__TCA_SKBEDIT_MAX - 1)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index fcfeeaf..5fca32b 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -43,8 +43,12 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 	if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
 	    skb->dev->real_num_tx_queues > d->queue_mapping)
 		skb_set_queue_mapping(skb, d->queue_mapping);
-	if (d->flags & SKBEDIT_F_MARK)
-		skb->mark = d->mark;
+	if (d->flags & SKBEDIT_F_MARK) {
+		/* unset all the bits in the mask */
+		skb->mark = skb->mark & ~d->mask;
+		/* assign the mark value for the masked bit only */
+		skb->mark |= d->mark & d->mask;
+	}
 
 	spin_unlock(&d->tcf_lock);
 	return d->tcf_action;
@@ -55,6 +59,7 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
 	[TCA_SKBEDIT_PRIORITY]		= { .len = sizeof(u32) },
 	[TCA_SKBEDIT_QUEUE_MAPPING]	= { .len = sizeof(u16) },
 	[TCA_SKBEDIT_MARK]		= { .len = sizeof(u32) },
+	[TCA_SKBEDIT_MASK]		= { .len = sizeof(u32) },
 };
 
 static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
@@ -64,7 +69,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 	struct nlattr *tb[TCA_SKBEDIT_MAX + 1];
 	struct tc_skbedit *parm;
 	struct tcf_skbedit *d;
-	u32 flags = 0, *priority = NULL, *mark = NULL;
+	u32 flags = 0, *priority = NULL, *mark = NULL, *mask = NULL;
 	u16 *queue_mapping = NULL;
 	int ret = 0, err;
 
@@ -93,6 +98,11 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 		mark = nla_data(tb[TCA_SKBEDIT_MARK]);
 	}
 
+	if (tb[TCA_SKBEDIT_MASK] != NULL) {
+		flags |= SKBEDIT_F_MASK;
+		mask = nla_data(tb[TCA_SKBEDIT_MASK]);
+	}
+
 	if (!flags)
 		return -EINVAL;
 
@@ -123,6 +133,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 		d->queue_mapping = *queue_mapping;
 	if (flags & SKBEDIT_F_MARK)
 		d->mark = *mark;
+	/* default behaviour is to use all the bits */
+	d->mask = 0xffffffff;
+	if (flags & SKBEDIT_F_MASK)
+		d->mask = *mask;
 
 	d->tcf_action = parm->action;
 
-- 
1.8.5.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbedit: allow the user to specify bitmask for mark
  2014-07-21  8:35 [PATCH] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
@ 2014-07-21 10:43 ` Jamal Hadi Salim
  2014-07-21 12:04 ` Sergei Shtylyov
  2014-07-21 12:31 ` Eric Dumazet
  2 siblings, 0 replies; 6+ messages in thread
From: Jamal Hadi Salim @ 2014-07-21 10:43 UTC (permalink / raw)
  To: Antonio Quartulli, davem; +Cc: netdev, Antonio Quartulli

On 07/21/14 04:35, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
>
> The user may want to use only some bits of the skb mark in
> his skbedit rules because the remaining part might be used by
> something else.
>
> Introduce the "mask" parameter to the skbedit actor in order
> to implement such functionality.
>
> When the mask is specified, only those bits selected by the
> latter are altered really changed by the actor, while the
> rest is left untouched.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>

Looks good!

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbedit: allow the user to specify bitmask for mark
  2014-07-21  8:35 [PATCH] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
  2014-07-21 10:43 ` Jamal Hadi Salim
@ 2014-07-21 12:04 ` Sergei Shtylyov
  2014-07-21 12:08   ` Antonio Quartulli
  2014-07-21 12:31 ` Eric Dumazet
  2 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2014-07-21 12:04 UTC (permalink / raw)
  To: Antonio Quartulli, davem; +Cc: netdev, Antonio Quartulli, Jamal Hadi Salim

Hello.

On 21-07-2014 12:35, Antonio Quartulli wrote:

> From: Antonio Quartulli <antonio@open-mesh.com>

> The user may want to use only some bits of the skb mark in
> his skbedit rules because the remaining part might be used by
> something else.

> Introduce the "mask" parameter to the skbedit actor in order
> to implement such functionality.

> When the mask is specified, only those bits selected by the
> latter are altered really changed by the actor, while the
> rest is left untouched.

> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
[...]

> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index fcfeeaf..5fca32b 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -43,8 +43,12 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
>   	if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
>   	    skb->dev->real_num_tx_queues > d->queue_mapping)
>   		skb_set_queue_mapping(skb, d->queue_mapping);
> -	if (d->flags & SKBEDIT_F_MARK)
> -		skb->mark = d->mark;
> +	if (d->flags & SKBEDIT_F_MARK) {
> +		/* unset all the bits in the mask */
> +		skb->mark = skb->mark & ~d->mask;

    Why not 'skb->mark &= ~d->mask;'?

> +		/* assign the mark value for the masked bit only */
> +		skb->mark |= d->mark & d->mask;
> +	}
>
>   	spin_unlock(&d->tcf_lock);
>   	return d->tcf_action;
[...]

WBR, Sergei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbedit: allow the user to specify bitmask for mark
  2014-07-21 12:04 ` Sergei Shtylyov
@ 2014-07-21 12:08   ` Antonio Quartulli
  0 siblings, 0 replies; 6+ messages in thread
From: Antonio Quartulli @ 2014-07-21 12:08 UTC (permalink / raw)
  To: Sergei Shtylyov, davem; +Cc: netdev, Jamal Hadi Salim

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]



On 21/07/14 14:04, Sergei Shtylyov wrote:
>> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
>> index fcfeeaf..5fca32b 100644
>> --- a/net/sched/act_skbedit.c
>> +++ b/net/sched/act_skbedit.c
>> @@ -43,8 +43,12 @@ static int tcf_skbedit(struct sk_buff *skb, const
>> struct tc_action *a,
>>       if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
>>           skb->dev->real_num_tx_queues > d->queue_mapping)
>>           skb_set_queue_mapping(skb, d->queue_mapping);
>> -    if (d->flags & SKBEDIT_F_MARK)
>> -        skb->mark = d->mark;
>> +    if (d->flags & SKBEDIT_F_MARK) {
>> +        /* unset all the bits in the mask */
>> +        skb->mark = skb->mark & ~d->mask;
> 
>    Why not 'skb->mark &= ~d->mask;'?


No real reason :) Shall we always use the compressed version of this
operator when possible ?


-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbedit: allow the user to specify bitmask for mark
  2014-07-21  8:35 [PATCH] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
  2014-07-21 10:43 ` Jamal Hadi Salim
  2014-07-21 12:04 ` Sergei Shtylyov
@ 2014-07-21 12:31 ` Eric Dumazet
  2014-07-21 12:45   ` Antonio Quartulli
  2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-07-21 12:31 UTC (permalink / raw)
  To: Antonio Quartulli; +Cc: davem, netdev, Antonio Quartulli, Jamal Hadi Salim

On Mon, 2014-07-21 at 10:35 +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> The user may want to use only some bits of the skb mark in
> his skbedit rules because the remaining part might be used by
> something else.
> 
> Introduce the "mask" parameter to the skbedit actor in order
> to implement such functionality.
> 
> When the mask is specified, only those bits selected by the
> latter are altered really changed by the actor, while the
> rest is left untouched.
> 
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  include/net/tc_act/tc_skbedit.h        |  1 +
>  include/uapi/linux/tc_act/tc_skbedit.h |  2 ++
>  net/sched/act_skbedit.c                | 20 +++++++++++++++++---
>  3 files changed, 20 insertions(+), 3 deletions(-)


You forgot to update tcf_skbedit_dump()

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] skbedit: allow the user to specify bitmask for mark
  2014-07-21 12:31 ` Eric Dumazet
@ 2014-07-21 12:45   ` Antonio Quartulli
  0 siblings, 0 replies; 6+ messages in thread
From: Antonio Quartulli @ 2014-07-21 12:45 UTC (permalink / raw)
  To: Eric Dumazet, Antonio Quartulli; +Cc: davem, netdev, Jamal Hadi Salim

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]



On 21/07/14 14:31, Eric Dumazet wrote:
> On Mon, 2014-07-21 at 10:35 +0200, Antonio Quartulli wrote:
>> From: Antonio Quartulli <antonio@open-mesh.com>
>>
>> The user may want to use only some bits of the skb mark in
>> his skbedit rules because the remaining part might be used by
>> something else.
>>
>> Introduce the "mask" parameter to the skbedit actor in order
>> to implement such functionality.
>>
>> When the mask is specified, only those bits selected by the
>> latter are altered really changed by the actor, while the
>> rest is left untouched.
>>
>> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
>> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
>> ---
>>  include/net/tc_act/tc_skbedit.h        |  1 +
>>  include/uapi/linux/tc_act/tc_skbedit.h |  2 ++
>>  net/sched/act_skbedit.c                | 20 +++++++++++++++++---
>>  3 files changed, 20 insertions(+), 3 deletions(-)
> 
> 
> You forgot to update tcf_skbedit_dump()
> 

Oh right! Thanks Eric.
I'll send v2

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-07-21 12:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21  8:35 [PATCH] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
2014-07-21 10:43 ` Jamal Hadi Salim
2014-07-21 12:04 ` Sergei Shtylyov
2014-07-21 12:08   ` Antonio Quartulli
2014-07-21 12:31 ` Eric Dumazet
2014-07-21 12:45   ` Antonio Quartulli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).