* [PATCH net-next 0/3] net/sched: act_pedit: Support using offset relative to the conventional network headers
@ 2016-11-30 9:09 Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 1/3] net/skbuff: Introduce skb_mac_offset() Amir Vadai
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Amir Vadai @ 2016-11-30 9:09 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jamal Hadi Salim, Or Gerlitz, Hadar Har-Zion, Amir Vadai
Hi,
Patch 1/3 ("net/skbuff: Introduce skb_mac_offset()") adds a utility function to
get mac header offset.
Patch 2/3 ("net/act_pedit: Support using offset relative to the conventional
network headers") extends pedit to enable the user to set offset relative to
MAC/IPv4/IPv6/TCP network headers.
This would enable to work with more complex header schemes (vs the simple IPv4
case) where setting a fixed offset relative to the network header is not
enough. It is also forward looking to enable hardware offloading of pedit more
easier.
The header type is embedded in the 8 MSB of the u32 key->shift which
were never used till now. Therefore backward compatibility is being
kept.
Patch 3/3 ("net/act_pedit: Introduce 'add' operation") add a new operation to
increase the value of a header field. The operation is passed on another free
8bit in the key->shift.
Usage example:
$ tc filter add dev enp0s9 protocol ip parent ffff: \
flower \
ip_proto tcp \
src_port 80 \
action \
pedit munge ip ttl add 0xff \
pedit munge tcp dport set 8080 \
pipe action mirred egress redirect dev veth0
Will forward traffic with tcp dport 80, and modify the destination port to
8080, and decrease the ttl by 1.
I've uploaded a draft for the userspace [2] to make it easier to review and
test the patchset.
The patchset will conflict if already accepted patch [1] from net is missing.
It was applied and tested with [1] on top of commit 93ba22225504 ("hv_netvsc:
remove excessive logging on MTU change").
[1] - 95c2027bfeda ("net/sched: pedit: make sure that offset is valid")
[2] - git: https://bitbucket.org/av42/iproute2.git
branch: pedit
Thanks,
Amir
Amir Vadai (3):
net/skbuff: Introduce skb_mac_offset()
net/act_pedit: Support using offset relative to the conventional
network headers
net/act_pedit: Introduce 'add' operation
include/linux/skbuff.h | 5 +++
include/uapi/linux/tc_act/tc_pedit.h | 27 ++++++++++++
net/sched/act_pedit.c | 81 ++++++++++++++++++++++++++++++------
3 files changed, 100 insertions(+), 13 deletions(-)
--
2.10.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 1/3] net/skbuff: Introduce skb_mac_offset()
2016-11-30 9:09 [PATCH net-next 0/3] net/sched: act_pedit: Support using offset relative to the conventional network headers Amir Vadai
@ 2016-11-30 9:09 ` Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 3/3] net/act_pedit: Introduce 'add' operation Amir Vadai
2 siblings, 0 replies; 7+ messages in thread
From: Amir Vadai @ 2016-11-30 9:09 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jamal Hadi Salim, Or Gerlitz, Hadar Har-Zion, Amir Vadai
Introduce skb_mac_offset() that could be used to get mac header offset.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
include/linux/skbuff.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9c535fbccf2c..395eb5111df0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2169,6 +2169,11 @@ static inline unsigned char *skb_mac_header(const struct sk_buff *skb)
return skb->head + skb->mac_header;
}
+static inline int skb_mac_offset(const struct sk_buff *skb)
+{
+ return skb_mac_header(skb) - skb->data;
+}
+
static inline int skb_mac_header_was_set(const struct sk_buff *skb)
{
return skb->mac_header != (typeof(skb->mac_header))~0U;
--
2.10.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers
2016-11-30 9:09 [PATCH net-next 0/3] net/sched: act_pedit: Support using offset relative to the conventional network headers Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 1/3] net/skbuff: Introduce skb_mac_offset() Amir Vadai
@ 2016-11-30 9:09 ` Amir Vadai
2016-12-01 19:41 ` David Miller
2016-11-30 9:09 ` [PATCH net-next 3/3] net/act_pedit: Introduce 'add' operation Amir Vadai
2 siblings, 1 reply; 7+ messages in thread
From: Amir Vadai @ 2016-11-30 9:09 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jamal Hadi Salim, Or Gerlitz, Hadar Har-Zion, Amir Vadai
Extend pedit to enable the user using offset relative to network
headers. This change would enable to work with more complex header
schemes (vs the simple IPv4 case) where setting a fixed offset relative
to the network header is not enough. It is also forward looking to
enable hardware offloading of pedit more easier.
The header type is embedded in the 8 MSB of the u32 key->shift which
were never used till now. Therefore backward compatibility is being
kept.
Usage example:
$ tc filter add dev enp0s9 protocol ip parent ffff: \
flower \
ip_proto tcp \
src_port 80 \
action pedit munge tcp dport set 8080 pipe \
action mirred egress redirect dev veth0
Will forward traffic to tcp port 80, and modify the destination port to
8080.
hange-Id: Ibd7bbbe0b8c2f6adae0591868bb6892c55e75732
Signed-off-by: Amir Vadai <amir@vadai.me>
---
include/uapi/linux/tc_act/tc_pedit.h | 17 ++++++++++
net/sched/act_pedit.c | 65 +++++++++++++++++++++++++++++-------
2 files changed, 70 insertions(+), 12 deletions(-)
diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h
index 6389959a5157..604e6729ad38 100644
--- a/include/uapi/linux/tc_act/tc_pedit.h
+++ b/include/uapi/linux/tc_act/tc_pedit.h
@@ -32,4 +32,21 @@ struct tc_pedit_sel {
};
#define tc_pedit tc_pedit_sel
+#define PEDIT_TYPE_SHIFT 24
+#define PEDIT_TYPE_MASK 0xff
+
+#define PEDIT_TYPE_GET(_val) \
+ (((_val) >> PEDIT_TYPE_SHIFT) & PEDIT_TYPE_MASK)
+#define PEDIT_SHIFT_GET(_val) ((_val) & 0xff)
+
+enum pedit_header_type {
+ PEDIT_HDR_TYPE_RAW = 0,
+
+ PEDIT_HDR_TYPE_ETH = 1,
+ PEDIT_HDR_TYPE_IP4 = 2,
+ PEDIT_HDR_TYPE_IP6 = 3,
+ PEDIT_HDR_TYPE_TCP = 4,
+ PEDIT_HDR_TYPE_UDP = 5,
+};
+
#endif
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index b27c4daec88f..4b9c7184c752 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -119,18 +119,45 @@ static bool offset_valid(struct sk_buff *skb, int offset)
return true;
}
+static int pedit_skb_hdr_offset(struct sk_buff *skb,
+ enum pedit_header_type htype, int *hoffset)
+{
+ int ret = -1;
+
+ switch (htype) {
+ case PEDIT_HDR_TYPE_ETH:
+ if (skb_mac_header_was_set(skb)) {
+ *hoffset = skb_mac_offset(skb);
+ ret = 0;
+ }
+ break;
+ case PEDIT_HDR_TYPE_RAW:
+ case PEDIT_HDR_TYPE_IP4:
+ case PEDIT_HDR_TYPE_IP6:
+ *hoffset = skb_network_offset(skb);
+ ret = 0;
+ break;
+ case PEDIT_HDR_TYPE_TCP:
+ case PEDIT_HDR_TYPE_UDP:
+ if (skb_transport_header_was_set(skb)) {
+ *hoffset = skb_transport_offset(skb);
+ ret = 0;
+ }
+ break;
+ };
+
+ return ret;
+}
+
static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
struct tcf_result *res)
{
struct tcf_pedit *p = to_pedit(a);
int i;
- unsigned int off;
if (skb_unclone(skb, GFP_ATOMIC))
return p->tcf_action;
- off = skb_network_offset(skb);
-
spin_lock(&p->tcf_lock);
tcf_lastuse_update(&p->tcf_tm);
@@ -141,20 +168,32 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
u32 *ptr, _data;
int offset = tkey->off;
+ int hoffset;
+ int rc;
+ enum pedit_header_type htype =
+ PEDIT_TYPE_GET(tkey->shift);
+
+ rc = pedit_skb_hdr_offset(skb, htype, &hoffset);
+ if (rc) {
+ pr_info("tc filter pedit bad header type specified (0x%x)\n",
+ htype);
+ goto bad;
+ }
if (tkey->offmask) {
char *d, _d;
- if (!offset_valid(skb, off + tkey->at)) {
+ if (!offset_valid(skb, hoffset + tkey->at)) {
pr_info("tc filter pedit 'at' offset %d out of bounds\n",
- off + tkey->at);
+ hoffset + tkey->at);
goto bad;
}
- d = skb_header_pointer(skb, off + tkey->at, 1,
- &_d);
+ d = skb_header_pointer(skb,
+ hoffset + tkey->at,
+ 1, &_d);
if (!d)
goto bad;
- offset += (*d & tkey->offmask) >> tkey->shift;
+ offset += (*d & tkey->offmask) >> PEDIT_SHIFT_GET(tkey->shift);
}
if (offset % 4) {
@@ -163,19 +202,21 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
goto bad;
}
- if (!offset_valid(skb, off + offset)) {
+ if (!offset_valid(skb, hoffset + offset)) {
pr_info("tc filter pedit offset %d out of bounds\n",
- offset);
+ hoffset + offset);
goto bad;
}
- ptr = skb_header_pointer(skb, off + offset, 4, &_data);
+ ptr = skb_header_pointer(skb,
+ hoffset + offset,
+ 4, &_data);
if (!ptr)
goto bad;
/* just do it, baby */
*ptr = ((*ptr & tkey->mask) ^ tkey->val);
if (ptr == &_data)
- skb_store_bits(skb, off + offset, ptr, 4);
+ skb_store_bits(skb, hoffset + offset, ptr, 4);
}
goto done;
--
2.10.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 3/3] net/act_pedit: Introduce 'add' operation
2016-11-30 9:09 [PATCH net-next 0/3] net/sched: act_pedit: Support using offset relative to the conventional network headers Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 1/3] net/skbuff: Introduce skb_mac_offset() Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers Amir Vadai
@ 2016-11-30 9:09 ` Amir Vadai
2 siblings, 0 replies; 7+ messages in thread
From: Amir Vadai @ 2016-11-30 9:09 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jamal Hadi Salim, Or Gerlitz, Hadar Har-Zion, Amir Vadai
This command could be useful to inc/dec fields.
For example, to forward any TCP packet and decrease its TTL:
$ tc filter add dev enp0s9 protocol ip parent ffff: \
flower ip_proto tcp \
action pedit munge ip ttl add 0xff pipe \
action mirred egress redirect dev veth0
In the example above, adding 0xff to this u8 field is actually
decreasing it by one, since the operation is masked.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
include/uapi/linux/tc_act/tc_pedit.h | 10 ++++++++++
net/sched/act_pedit.c | 16 +++++++++++++++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h
index 604e6729ad38..80028cd0bb1b 100644
--- a/include/uapi/linux/tc_act/tc_pedit.h
+++ b/include/uapi/linux/tc_act/tc_pedit.h
@@ -35,8 +35,13 @@ struct tc_pedit_sel {
#define PEDIT_TYPE_SHIFT 24
#define PEDIT_TYPE_MASK 0xff
+#define PEDIT_CMD_SHIFT 16
+#define PEDIT_CMD_MASK 0xff
+
#define PEDIT_TYPE_GET(_val) \
(((_val) >> PEDIT_TYPE_SHIFT) & PEDIT_TYPE_MASK)
+#define PEDIT_CMD_GET(_val) \
+ (((_val) >> PEDIT_CMD_SHIFT) & PEDIT_CMD_MASK)
#define PEDIT_SHIFT_GET(_val) ((_val) & 0xff)
enum pedit_header_type {
@@ -49,4 +54,9 @@ enum pedit_header_type {
PEDIT_HDR_TYPE_UDP = 5,
};
+enum pedit_cmd {
+ PEDIT_CMD_SET = 0,
+ PEDIT_CMD_ADD = 1,
+};
+
#endif
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 4b9c7184c752..aa137d51bf7f 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -169,6 +169,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
u32 *ptr, _data;
int offset = tkey->off;
int hoffset;
+ u32 val;
int rc;
enum pedit_header_type htype =
PEDIT_TYPE_GET(tkey->shift);
@@ -214,7 +215,20 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
if (!ptr)
goto bad;
/* just do it, baby */
- *ptr = ((*ptr & tkey->mask) ^ tkey->val);
+ switch (PEDIT_CMD_GET(tkey->shift)) {
+ case PEDIT_CMD_SET:
+ val = tkey->val;
+ break;
+ case PEDIT_CMD_ADD:
+ val = (*ptr + tkey->val) & ~tkey->mask;
+ break;
+ default:
+ pr_info("tc filter pedit bad command (%d)\n",
+ PEDIT_CMD_GET(tkey->shift));
+ goto bad;
+ }
+
+ *ptr = ((*ptr & tkey->mask) ^ val);
if (ptr == &_data)
skb_store_bits(skb, hoffset + offset, ptr, 4);
}
--
2.10.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers
2016-11-30 9:09 ` [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers Amir Vadai
@ 2016-12-01 19:41 ` David Miller
2016-12-02 10:40 ` Amir Vadai
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2016-12-01 19:41 UTC (permalink / raw)
To: amir; +Cc: netdev, jhs, ogerlitz, hadarh
From: Amir Vadai <amir@vadai.me>
Date: Wed, 30 Nov 2016 11:09:27 +0200
> @@ -119,18 +119,45 @@ static bool offset_valid(struct sk_buff *skb, int offset)
> return true;
> }
>
> +static int pedit_skb_hdr_offset(struct sk_buff *skb,
> + enum pedit_header_type htype, int *hoffset)
> +{
> + int ret = -1;
> +
> + switch (htype) {
> + case PEDIT_HDR_TYPE_ETH:
> + if (skb_mac_header_was_set(skb)) {
> + *hoffset = skb_mac_offset(skb);
> + ret = 0;
> + }
> + break;
> + case PEDIT_HDR_TYPE_RAW:
> + case PEDIT_HDR_TYPE_IP4:
> + case PEDIT_HDR_TYPE_IP6:
> + *hoffset = skb_network_offset(skb);
> + ret = 0;
> + break;
> + case PEDIT_HDR_TYPE_TCP:
> + case PEDIT_HDR_TYPE_UDP:
> + if (skb_transport_header_was_set(skb)) {
> + *hoffset = skb_transport_offset(skb);
> + ret = 0;
> + }
> + break;
> + };
> +
> + return ret;
> +}
> +
The only distinction between the cases is "L2", "L3", and "L4".
Therefore I don't see any reason to break it down into IP4 vs. IP6 vs.
RAW, for example. They all map to the same thing.
So why not just have PEDIT_HDR_TYPE_L2, PEDIT_HDR_TYPE_L3, and
PEDIT_HDR_TYPE_L4? It definitely seems more straightforward
and cleaner that way.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers
2016-12-01 19:41 ` David Miller
@ 2016-12-02 10:40 ` Amir Vadai
2016-12-04 21:55 ` Or Gerlitz
0 siblings, 1 reply; 7+ messages in thread
From: Amir Vadai @ 2016-12-02 10:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jhs, ogerlitz, hadarh
On Thu, Dec 01, 2016 at 02:41:14PM -0500, David Miller wrote:
> From: Amir Vadai <amir@vadai.me>
> Date: Wed, 30 Nov 2016 11:09:27 +0200
>
> > @@ -119,18 +119,45 @@ static bool offset_valid(struct sk_buff *skb, int offset)
> > return true;
> > }
> >
> > +static int pedit_skb_hdr_offset(struct sk_buff *skb,
> > + enum pedit_header_type htype, int *hoffset)
> > +{
> > + int ret = -1;
> > +
> > + switch (htype) {
> > + case PEDIT_HDR_TYPE_ETH:
> > + if (skb_mac_header_was_set(skb)) {
> > + *hoffset = skb_mac_offset(skb);
> > + ret = 0;
> > + }
> > + break;
> > + case PEDIT_HDR_TYPE_RAW:
> > + case PEDIT_HDR_TYPE_IP4:
> > + case PEDIT_HDR_TYPE_IP6:
> > + *hoffset = skb_network_offset(skb);
> > + ret = 0;
> > + break;
> > + case PEDIT_HDR_TYPE_TCP:
> > + case PEDIT_HDR_TYPE_UDP:
> > + if (skb_transport_header_was_set(skb)) {
> > + *hoffset = skb_transport_offset(skb);
> > + ret = 0;
> > + }
> > + break;
> > + };
> > +
> > + return ret;
> > +}
> > +
>
> The only distinction between the cases is "L2", "L3", and "L4".
>
> Therefore I don't see any reason to break it down into IP4 vs. IP6 vs.
> RAW, for example. They all map to the same thing.
>
> So why not just have PEDIT_HDR_TYPE_L2, PEDIT_HDR_TYPE_L3, and
> PEDIT_HDR_TYPE_L4? It definitely seems more straightforward
> and cleaner that way.
Yeh, is isn't by mistake. The next step will be to implement hardware
offloading of the action, and for that we would like to keep the
information about the specific header type.
>
> Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers
2016-12-02 10:40 ` Amir Vadai
@ 2016-12-04 21:55 ` Or Gerlitz
0 siblings, 0 replies; 7+ messages in thread
From: Or Gerlitz @ 2016-12-04 21:55 UTC (permalink / raw)
To: David Miller
Cc: Linux Netdev List, Jamal Hadi Salim, Hadar Hen Zion, Jiri Pirko
On Fri, Dec 2, 2016 at 12:40 PM, Amir Vadai <amir@vadai.me> wrote:
> On Thu, Dec 01, 2016 at 02:41:14PM -0500, David Miller wrote:
>> From: Amir Vadai <amir@vadai.me>
>> Date: Wed, 30 Nov 2016 11:09:27 +0200
>> > +static int pedit_skb_hdr_offset(struct sk_buff *skb,
>> > + enum pedit_header_type htype, int *hoffset)
>> > +{
>> > + int ret = -1;
>> > +
>> > + switch (htype) {
>> > + case PEDIT_HDR_TYPE_ETH:
>> > + if (skb_mac_header_was_set(skb)) {
>> > + *hoffset = skb_mac_offset(skb);
>> > + ret = 0;
>> > + }
>> > + break;
>> > + case PEDIT_HDR_TYPE_RAW:
>> > + case PEDIT_HDR_TYPE_IP4:
>> > + case PEDIT_HDR_TYPE_IP6:
>> > + *hoffset = skb_network_offset(skb);
>> > + ret = 0;
>> > + break;
>> > + case PEDIT_HDR_TYPE_TCP:
>> > + case PEDIT_HDR_TYPE_UDP:
>> > + if (skb_transport_header_was_set(skb)) {
>> > + *hoffset = skb_transport_offset(skb);
>> > + ret = 0;
>> > + }
>> > + break;
>> > + };
>> > +
>> > + return ret;
>> > +}
>> > +
>> The only distinction between the cases is "L2", "L3", and "L4".
>> Therefore I don't see any reason to break it down into IP4 vs. IP6 vs.
>> RAW, for example. They all map to the same thing.
>> So why not just have PEDIT_HDR_TYPE_L2, PEDIT_HDR_TYPE_L3, and
>> PEDIT_HDR_TYPE_L4? It definitely seems more straightforward
>> and cleaner that way.
> Yeh, is isn't by mistake. The next step will be to implement hardware
> offloading of the action, and for that we would like to keep the
> information about the specific header type.
Hi Dave,
I see that this patch is marked as "Changes Requested" @ your patchworks.
Just wanted to make a note as Amir explained here and as mentioned on
the change log, this was done in purpose, as heads up for HW offloads.
Typically HW APIs would let you do things also based on header type
they have parsed, etc, so that's why we added this small redundancy
e.g of IPv4/IPv6 header ID instead of network header ID - while SW
wise both IPv4/IPv6 are using the same code path, for HW offloads, the
HW driver could choose to use the IPv4/IPv6 header ID info.
Or.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-12-04 21:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-30 9:09 [PATCH net-next 0/3] net/sched: act_pedit: Support using offset relative to the conventional network headers Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 1/3] net/skbuff: Introduce skb_mac_offset() Amir Vadai
2016-11-30 9:09 ` [PATCH net-next 2/3] net/act_pedit: Support using offset relative to the conventional network headers Amir Vadai
2016-12-01 19:41 ` David Miller
2016-12-02 10:40 ` Amir Vadai
2016-12-04 21:55 ` Or Gerlitz
2016-11-30 9:09 ` [PATCH net-next 3/3] net/act_pedit: Introduce 'add' operation Amir Vadai
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).