* [PATCHv2] skbedit: allow the user to specify bitmask for mark
@ 2014-07-23 8:03 Antonio Quartulli
2014-07-23 13:16 ` Jamal Hadi Salim
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Antonio Quartulli @ 2014-07-23 8:03 UTC (permalink / raw)
To: davem
Cc: netdev, sergei.shtylyov, eric.dumazet, 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>
---
Jamal, I'm not adding your Signed-off because the patches has new part with
respect to v1.
Changes from v1:
- use '&=' tcf_skbedit() to clean the mark
- extend tcf_skbedit_dump() in order to send the mask as well
include/net/tc_act/tc_skbedit.h | 1 +
include/uapi/linux/tc_act/tc_skbedit.h | 2 ++
net/sched/act_skbedit.c | 24 +++++++++++++++++++++---
3 files changed, 24 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..6111b06 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 &= ~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;
@@ -160,6 +174,10 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
nla_put(skb, TCA_SKBEDIT_MARK, sizeof(d->mark),
&d->mark))
goto nla_put_failure;
+ if ((d->flags & SKBEDIT_F_MASK) &&
+ nla_put(skb, TCA_SKBEDIT_MASK, sizeof(d->mask),
+ &d->mark))
+ goto nla_put_failure;
t.install = jiffies_to_clock_t(jiffies - d->tcf_tm.install);
t.lastuse = jiffies_to_clock_t(jiffies - d->tcf_tm.lastuse);
t.expires = jiffies_to_clock_t(d->tcf_tm.expires);
--
1.8.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCHv2] skbedit: allow the user to specify bitmask for mark
2014-07-23 8:03 [PATCHv2] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
@ 2014-07-23 13:16 ` Jamal Hadi Salim
2014-07-23 16:39 ` Eric Dumazet
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jamal Hadi Salim @ 2014-07-23 13:16 UTC (permalink / raw)
To: Antonio Quartulli, davem
Cc: netdev, sergei.shtylyov, eric.dumazet, Antonio Quartulli
On 07/23/14 04:03, 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>
> ---
>
> Jamal, I'm not adding your Signed-off because the patches has new part with
> respect to v1.
>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] skbedit: allow the user to specify bitmask for mark
2014-07-23 8:03 [PATCHv2] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
2014-07-23 13:16 ` Jamal Hadi Salim
@ 2014-07-23 16:39 ` Eric Dumazet
2014-07-23 17:15 ` Cong Wang
2014-07-23 17:16 ` Cong Wang
3 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2014-07-23 16:39 UTC (permalink / raw)
To: Antonio Quartulli
Cc: davem, netdev, sergei.shtylyov, Antonio Quartulli,
Jamal Hadi Salim
On Wed, 2014-07-23 at 10:03 +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
>
>
> @@ -160,6 +174,10 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
> nla_put(skb, TCA_SKBEDIT_MARK, sizeof(d->mark),
> &d->mark))
> goto nla_put_failure;
> + if ((d->flags & SKBEDIT_F_MASK) &&
> + nla_put(skb, TCA_SKBEDIT_MASK, sizeof(d->mask),
> + &d->mark))
typo here : d->mask is better.
Or even better :
nla_put_u32(skb, TCA_SKBEDIT_MASK, d->mask)
> + goto nla_put_failure;
> t.install = jiffies_to_clock_t(jiffies - d->tcf_tm.install);
> t.lastuse = jiffies_to_clock_t(jiffies - d->tcf_tm.lastuse);
> t.expires = jiffies_to_clock_t(d->tcf_tm.expires);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] skbedit: allow the user to specify bitmask for mark
2014-07-23 8:03 [PATCHv2] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
2014-07-23 13:16 ` Jamal Hadi Salim
2014-07-23 16:39 ` Eric Dumazet
@ 2014-07-23 17:15 ` Cong Wang
2014-07-23 17:16 ` Cong Wang
3 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2014-07-23 17:15 UTC (permalink / raw)
To: Antonio Quartulli
Cc: David Miller, netdev, sergei.shtylyov, Eric Dumazet,
Antonio Quartulli, Jamal Hadi Salim
On Wed, Jul 23, 2014 at 1:03 AM, Antonio Quartulli
<antonio@meshcoding.com> 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>
> ---
>
> Jamal, I'm not adding your Signed-off because the patches has new part with
> respect to v1.
>
>
> Changes from v1:
> - use '&=' tcf_skbedit() to clean the mark
> - extend tcf_skbedit_dump() in order to send the mask as well
>
>
>
>
>
> include/net/tc_act/tc_skbedit.h | 1 +
> include/uapi/linux/tc_act/tc_skbedit.h | 2 ++
> net/sched/act_skbedit.c | 24 +++++++++++++++++++++---
> 3 files changed, 24 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..6111b06 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 &= ~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;
>
> @@ -160,6 +174,10 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
> nla_put(skb, TCA_SKBEDIT_MARK, sizeof(d->mark),
> &d->mark))
> goto nla_put_failure;
> + if ((d->flags & SKBEDIT_F_MASK) &&
> + nla_put(skb, TCA_SKBEDIT_MASK, sizeof(d->mask),
> + &d->mark))
> + goto nla_put_failure;
> t.install = jiffies_to_clock_t(jiffies - d->tcf_tm.install);
> t.lastuse = jiffies_to_clock_t(jiffies - d->tcf_tm.lastuse);
> t.expires = jiffies_to_clock_t(d->tcf_tm.expires);
> --
> 1.8.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCHv2] skbedit: allow the user to specify bitmask for mark
2014-07-23 8:03 [PATCHv2] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
` (2 preceding siblings ...)
2014-07-23 17:15 ` Cong Wang
@ 2014-07-23 17:16 ` Cong Wang
3 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2014-07-23 17:16 UTC (permalink / raw)
To: Antonio Quartulli
Cc: David Miller, netdev, sergei.shtylyov, Eric Dumazet,
Antonio Quartulli, Jamal Hadi Salim
On Wed, Jul 23, 2014 at 1:03 AM, Antonio Quartulli
<antonio@meshcoding.com> wrote:
> #define TCA_SKBEDIT_MAX (__TCA_SKBEDIT_MAX - 1)
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index fcfeeaf..6111b06 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 &= ~d->mask;
> + /* assign the mark value for the masked bit only */
> + skb->mark |= d->mark & d->mask;
The comments are useless, code is already clear.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-07-23 17:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-23 8:03 [PATCHv2] skbedit: allow the user to specify bitmask for mark Antonio Quartulli
2014-07-23 13:16 ` Jamal Hadi Salim
2014-07-23 16:39 ` Eric Dumazet
2014-07-23 17:15 ` Cong Wang
2014-07-23 17:16 ` Cong Wang
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).