* [PATCH net-next v2 1/3] openvswitch: mpls: set network header correctly on key extract
2016-09-30 17:08 [PATCH net-next v2 0/3] openvswitch: mpls fix and clean up Jiri Benc
@ 2016-09-30 17:08 ` Jiri Benc
2016-09-30 17:08 ` [PATCH net-next v2 2/3] mpls: move mpls_hdr to a common location Jiri Benc
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Jiri Benc @ 2016-09-30 17:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, pravin shelar
After the 48d2ab609b6b ("net: mpls: Fixups for GSO"), MPLS handling in
openvswitch was changed to have network header pointing to the start of the
MPLS headers and inner_network_header pointing after the MPLS headers.
However, key_extract was missed by the mentioned commit, causing incorrect
headers to be set when a MPLS packet just enters the bridge or after it is
recirculated.
Fixes: 48d2ab609b6b ("net: mpls: Fixups for GSO")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>
---
v2: no changes
---
net/openvswitch/flow.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 634cc10d6dee..c8c82e109c68 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -633,12 +633,7 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
} else if (eth_p_mpls(key->eth.type)) {
size_t stack_len = MPLS_HLEN;
- /* In the presence of an MPLS label stack the end of the L2
- * header and the beginning of the L3 header differ.
- *
- * Advance network_header to the beginning of the L3
- * header. mac_len corresponds to the end of the L2 header.
- */
+ skb_set_inner_network_header(skb, skb->mac_len);
while (1) {
__be32 lse;
@@ -646,12 +641,12 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
if (unlikely(error))
return 0;
- memcpy(&lse, skb_network_header(skb), MPLS_HLEN);
+ memcpy(&lse, skb_inner_network_header(skb), MPLS_HLEN);
if (stack_len == MPLS_HLEN)
memcpy(&key->mpls.top_lse, &lse, MPLS_HLEN);
- skb_set_network_header(skb, skb->mac_len + stack_len);
+ skb_set_inner_network_header(skb, skb->mac_len + stack_len);
if (lse & htonl(MPLS_LS_S_MASK))
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next v2 2/3] mpls: move mpls_hdr to a common location
2016-09-30 17:08 [PATCH net-next v2 0/3] openvswitch: mpls fix and clean up Jiri Benc
2016-09-30 17:08 ` [PATCH net-next v2 1/3] openvswitch: mpls: set network header correctly on key extract Jiri Benc
@ 2016-09-30 17:08 ` Jiri Benc
2016-09-30 17:14 ` David Ahern
2016-09-30 17:43 ` pravin shelar
2016-09-30 17:08 ` [PATCH net-next v2 3/3] openvswitch: use mpls_hdr Jiri Benc
2016-10-03 6:00 ` [PATCH net-next v2 0/3] openvswitch: mpls fix and clean up David Miller
3 siblings, 2 replies; 10+ messages in thread
From: Jiri Benc @ 2016-09-30 17:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, pravin shelar
This will be also used by openvswitch.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
v2: new in v2
---
include/net/mpls.h | 9 +++++++++
net/mpls/internal.h | 10 +---------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/net/mpls.h b/include/net/mpls.h
index 5b3b5addfb08..3ebbc0bb57ff 100644
--- a/include/net/mpls.h
+++ b/include/net/mpls.h
@@ -19,12 +19,21 @@
#define MPLS_HLEN 4
+struct mpls_shim_hdr {
+ __be32 label_stack_entry;
+};
+
static inline bool eth_p_mpls(__be16 eth_type)
{
return eth_type == htons(ETH_P_MPLS_UC) ||
eth_type == htons(ETH_P_MPLS_MC);
}
+static inline struct mpls_shim_hdr *mpls_hdr(const struct sk_buff *skb)
+{
+ return (struct mpls_shim_hdr *)skb_network_header(skb);
+}
+
/*
* For non-MPLS skbs this will correspond to the network header.
* For MPLS skbs it will be before the network_header as the MPLS
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index 732a5c17e986..bdfef6c3271a 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -1,9 +1,6 @@
#ifndef MPLS_INTERNAL_H
#define MPLS_INTERNAL_H
-
-struct mpls_shim_hdr {
- __be32 label_stack_entry;
-};
+#include <net/mpls.h>
struct mpls_entry_decoded {
u32 label;
@@ -93,11 +90,6 @@ struct mpls_route { /* next hop label forwarding entry */
#define endfor_nexthops(rt) }
-static inline struct mpls_shim_hdr *mpls_hdr(const struct sk_buff *skb)
-{
- return (struct mpls_shim_hdr *)skb_network_header(skb);
-}
-
static inline struct mpls_shim_hdr mpls_entry_encode(u32 label, unsigned ttl, unsigned tc, bool bos)
{
struct mpls_shim_hdr result;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next v2 3/3] openvswitch: use mpls_hdr
2016-09-30 17:08 [PATCH net-next v2 0/3] openvswitch: mpls fix and clean up Jiri Benc
2016-09-30 17:08 ` [PATCH net-next v2 1/3] openvswitch: mpls: set network header correctly on key extract Jiri Benc
2016-09-30 17:08 ` [PATCH net-next v2 2/3] mpls: move mpls_hdr to a common location Jiri Benc
@ 2016-09-30 17:08 ` Jiri Benc
2016-09-30 18:20 ` pravin shelar
2016-10-03 6:00 ` [PATCH net-next v2 0/3] openvswitch: mpls fix and clean up David Miller
3 siblings, 1 reply; 10+ messages in thread
From: Jiri Benc @ 2016-09-30 17:08 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, pravin shelar
skb_mpls_header is equivalent to mpls_hdr now. Use the existing helper
instead.
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
v2: using the existing mpls_hdr instead of skb_network_header
---
include/net/mpls.h | 12 ------------
net/openvswitch/actions.c | 24 ++++++++++++------------
2 files changed, 12 insertions(+), 24 deletions(-)
diff --git a/include/net/mpls.h b/include/net/mpls.h
index 3ebbc0bb57ff..1dbc669b770e 100644
--- a/include/net/mpls.h
+++ b/include/net/mpls.h
@@ -33,16 +33,4 @@ static inline struct mpls_shim_hdr *mpls_hdr(const struct sk_buff *skb)
{
return (struct mpls_shim_hdr *)skb_network_header(skb);
}
-
-/*
- * For non-MPLS skbs this will correspond to the network header.
- * For MPLS skbs it will be before the network_header as the MPLS
- * label stack lies between the end of the mac header and the network
- * header. That is, for MPLS skbs the end of the mac header
- * is the top of the MPLS label stack.
- */
-static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
-{
- return skb_mac_header(skb) + skb->mac_len;
-}
#endif
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 863e992dfbc0..4e03f64709bc 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -160,7 +160,7 @@ static void update_ethertype(struct sk_buff *skb, struct ethhdr *hdr,
static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
const struct ovs_action_push_mpls *mpls)
{
- __be32 *new_mpls_lse;
+ struct mpls_shim_hdr *new_mpls_lse;
/* Networking stack do not allow simultaneous Tunnel and MPLS GSO. */
if (skb->encapsulation)
@@ -180,8 +180,8 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
skb_reset_mac_header(skb);
skb_set_network_header(skb, skb->mac_len);
- new_mpls_lse = (__be32 *)skb_mpls_header(skb);
- *new_mpls_lse = mpls->mpls_lse;
+ new_mpls_lse = mpls_hdr(skb);
+ new_mpls_lse->label_stack_entry = mpls->mpls_lse;
skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
@@ -202,7 +202,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
if (unlikely(err))
return err;
- skb_postpull_rcsum(skb, skb_mpls_header(skb), MPLS_HLEN);
+ skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
skb->mac_len);
@@ -211,10 +211,10 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
skb_reset_mac_header(skb);
skb_set_network_header(skb, skb->mac_len);
- /* skb_mpls_header() is used to locate the ethertype
- * field correctly in the presence of VLAN tags.
+ /* mpls_hdr() is used to locate the ethertype field correctly in the
+ * presence of VLAN tags.
*/
- hdr = (struct ethhdr *)(skb_mpls_header(skb) - ETH_HLEN);
+ hdr = (struct ethhdr *)((void *)mpls_hdr(skb) - ETH_HLEN);
update_ethertype(skb, hdr, ethertype);
if (eth_p_mpls(skb->protocol))
skb->protocol = ethertype;
@@ -226,7 +226,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
const __be32 *mpls_lse, const __be32 *mask)
{
- __be32 *stack;
+ struct mpls_shim_hdr *stack;
__be32 lse;
int err;
@@ -234,16 +234,16 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
if (unlikely(err))
return err;
- stack = (__be32 *)skb_mpls_header(skb);
- lse = OVS_MASKED(*stack, *mpls_lse, *mask);
+ stack = mpls_hdr(skb);
+ lse = OVS_MASKED(stack->label_stack_entry, *mpls_lse, *mask);
if (skb->ip_summed == CHECKSUM_COMPLETE) {
- __be32 diff[] = { ~(*stack), lse };
+ __be32 diff[] = { ~(stack->label_stack_entry), lse };
skb->csum = ~csum_partial((char *)diff, sizeof(diff),
~skb->csum);
}
- *stack = lse;
+ stack->label_stack_entry = lse;
flow_key->mpls.top_lse = lse;
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 10+ messages in thread