* [PATCH v2 net-next 1/2] openvswitch: Add a missing break statement.
@ 2016-11-29 2:41 Jarno Rajahalme
2016-11-29 2:41 ` [PATCH v2 net-next 2/2] openvswitch: Fix skb->protocol for vlan frames Jarno Rajahalme
0 siblings, 1 reply; 4+ messages in thread
From: Jarno Rajahalme @ 2016-11-29 2:41 UTC (permalink / raw)
To: netdev; +Cc: jbenc, jarno
Add a break statement to prevent fall-through from
OVS_KEY_ATTR_ETHERNET to OVS_KEY_ATTR_TUNNEL. Without the break
actions setting ethernet addresses fail to validate with log messages
complaining about invalid tunnel attributes.
Fixes: 0a6410fbde ("openvswitch: netlink: support L3 packets")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Acked-by: Jiri Benc <jbenc@redhat.com>
---
v2: No change.
net/openvswitch/flow_netlink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index d19044f..c87d359 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2195,6 +2195,7 @@ static int validate_set(const struct nlattr *a,
case OVS_KEY_ATTR_ETHERNET:
if (mac_proto != MAC_PROTO_ETHERNET)
return -EINVAL;
+ break;
case OVS_KEY_ATTR_TUNNEL:
if (masked)
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 net-next 2/2] openvswitch: Fix skb->protocol for vlan frames.
2016-11-29 2:41 [PATCH v2 net-next 1/2] openvswitch: Add a missing break statement Jarno Rajahalme
@ 2016-11-29 2:41 ` Jarno Rajahalme
2016-11-29 7:21 ` Pravin Shelar
0 siblings, 1 reply; 4+ messages in thread
From: Jarno Rajahalme @ 2016-11-29 2:41 UTC (permalink / raw)
To: netdev; +Cc: jbenc, jarno
Do not set skb->protocol to be the ethertype of the L3 header, unless
the packet only has the L3 header. For a non-hardware offloaded VLAN
frame skb->protocol needs to be one of the VLAN ethertypes.
Any VLAN offloading is undone on the OVS netlink interface. Also any
VLAN tags added by userspace are non-offloaded.
Incorrect skb->protocol value on a full-size non-offloaded VLAN skb
causes packet drop due to failing MTU check, as the VLAN header should
not be counted in when considering MTU in ovs_vport_send().
Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
v2: Set skb->protocol when an ETH_P_TEB frame is received via ARPHRD_NONE
interface.
net/openvswitch/datapath.c | 1 -
net/openvswitch/flow.c | 30 ++++++++++++++++++++++--------
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 2d4c4d3..9c62b63 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -606,7 +606,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
rcu_assign_pointer(flow->sf_acts, acts);
packet->priority = flow->key.phy.priority;
packet->mark = flow->key.phy.skb_mark;
- packet->protocol = flow->key.eth.type;
rcu_read_lock();
dp = get_dp_rcu(net, ovs_header->dp_ifindex);
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 08aa926..b9aae99 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -477,12 +477,17 @@ static int parse_icmpv6(struct sk_buff *skb, struct sw_flow_key *key,
}
/**
- * key_extract - extracts a flow key from an Ethernet frame.
+ * key_extract - extracts a flow key from a packet with or without an
+ * Ethernet header.
* @skb: sk_buff that contains the frame, with skb->data pointing to the
- * Ethernet header
+ * beginning of the packet.
* @key: output flow key
*
- * The caller must ensure that skb->len >= ETH_HLEN.
+ * 'key->mac_proto' must be initialized to indicate the frame type.
+ * For an L3 frame 'key->mac_proto' must equal 'MAC_PROTO_NONE', and the
+ * caller must ensure that 'skb->protocol' is set to the ethertype of the L3
+ * header. Otherwise the presence of an Ethernet header is assumed and
+ * the caller must ensure that skb->len >= ETH_HLEN.
*
* Returns 0 if successful, otherwise a negative errno value.
*
@@ -498,8 +503,9 @@ static int parse_icmpv6(struct sk_buff *skb, struct sw_flow_key *key,
* of a correct length, otherwise the same as skb->network_header.
* For other key->eth.type values it is left untouched.
*
- * - skb->protocol: the type of the data starting at skb->network_header.
- * Equals to key->eth.type.
+ * - skb->protocol: For non-accelerated VLAN, one of the VLAN ether types,
+ * otherwise the same as key->eth.type, the ether type of the payload
+ * starting at skb->network_header.
*/
static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
{
@@ -518,6 +524,7 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return -EINVAL;
skb_reset_network_header(skb);
+ key->eth.type = skb->protocol;
} else {
eth = eth_hdr(skb);
ether_addr_copy(key->eth.src, eth->h_source);
@@ -531,15 +538,22 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
if (unlikely(parse_vlan(skb, key)))
return -ENOMEM;
- skb->protocol = parse_ethertype(skb);
- if (unlikely(skb->protocol == htons(0)))
+ key->eth.type = parse_ethertype(skb);
+ if (unlikely(key->eth.type == htons(0)))
return -ENOMEM;
+ if (skb->protocol == htons(ETH_P_TEB)) {
+ if (key->eth.vlan.tci & htons(VLAN_TAG_PRESENT)
+ && !skb_vlan_tag_present(skb))
+ skb->protocol = key->eth.vlan.tpid;
+ else
+ skb->protocol = key->eth.type;
+ }
+
skb_reset_network_header(skb);
__skb_push(skb, skb->data - skb_mac_header(skb));
}
skb_reset_mac_len(skb);
- key->eth.type = skb->protocol;
/* Network layer. */
if (key->eth.type == htons(ETH_P_IP)) {
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next 2/2] openvswitch: Fix skb->protocol for vlan frames.
2016-11-29 2:41 ` [PATCH v2 net-next 2/2] openvswitch: Fix skb->protocol for vlan frames Jarno Rajahalme
@ 2016-11-29 7:21 ` Pravin Shelar
2016-11-29 23:32 ` Jarno Rajahalme
0 siblings, 1 reply; 4+ messages in thread
From: Pravin Shelar @ 2016-11-29 7:21 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: Linux Kernel Network Developers, Jiri Benc
On Mon, Nov 28, 2016 at 6:41 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
> Do not set skb->protocol to be the ethertype of the L3 header, unless
> the packet only has the L3 header. For a non-hardware offloaded VLAN
> frame skb->protocol needs to be one of the VLAN ethertypes.
>
> Any VLAN offloading is undone on the OVS netlink interface. Also any
> VLAN tags added by userspace are non-offloaded.
>
> Incorrect skb->protocol value on a full-size non-offloaded VLAN skb
> causes packet drop due to failing MTU check, as the VLAN header should
> not be counted in when considering MTU in ovs_vport_send().
>
I think we should move to is_skb_forwardable() type of packet length
check in vport-send and get rid of skb-protocol checks altogether.
> Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets")
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---
> v2: Set skb->protocol when an ETH_P_TEB frame is received via ARPHRD_NONE
> interface.
>
> net/openvswitch/datapath.c | 1 -
> net/openvswitch/flow.c | 30 ++++++++++++++++++++++--------
> 2 files changed, 22 insertions(+), 9 deletions(-)
...
...
> @@ -531,15 +538,22 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
> if (unlikely(parse_vlan(skb, key)))
> return -ENOMEM;
>
> - skb->protocol = parse_ethertype(skb);
> - if (unlikely(skb->protocol == htons(0)))
> + key->eth.type = parse_ethertype(skb);
> + if (unlikely(key->eth.type == htons(0)))
> return -ENOMEM;
>
> + if (skb->protocol == htons(ETH_P_TEB)) {
> + if (key->eth.vlan.tci & htons(VLAN_TAG_PRESENT)
> + && !skb_vlan_tag_present(skb))
> + skb->protocol = key->eth.vlan.tpid;
> + else
> + skb->protocol = key->eth.type;
> + }
> +
I am not sure if this work in case of nested vlans.
Can we move skb-protocol assignment to parse_vlan() to avoid checking
for non-accelerated vlan case again here?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next 2/2] openvswitch: Fix skb->protocol for vlan frames.
2016-11-29 7:21 ` Pravin Shelar
@ 2016-11-29 23:32 ` Jarno Rajahalme
0 siblings, 0 replies; 4+ messages in thread
From: Jarno Rajahalme @ 2016-11-29 23:32 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Jiri Benc
> On Nov 28, 2016, at 11:21 PM, Pravin Shelar <pshelar@ovn.org> wrote:
>
> On Mon, Nov 28, 2016 at 6:41 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
>> Do not set skb->protocol to be the ethertype of the L3 header, unless
>> the packet only has the L3 header. For a non-hardware offloaded VLAN
>> frame skb->protocol needs to be one of the VLAN ethertypes.
>>
>> Any VLAN offloading is undone on the OVS netlink interface. Also any
>> VLAN tags added by userspace are non-offloaded.
>>
>> Incorrect skb->protocol value on a full-size non-offloaded VLAN skb
>> causes packet drop due to failing MTU check, as the VLAN header should
>> not be counted in when considering MTU in ovs_vport_send().
>>
> I think we should move to is_skb_forwardable() type of packet length
> check in vport-send and get rid of skb-protocol checks altogether.
>
I added a new patch to do this, thanks for the suggestion.
>> Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets")
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
>> v2: Set skb->protocol when an ETH_P_TEB frame is received via ARPHRD_NONE
>> interface.
>>
>> net/openvswitch/datapath.c | 1 -
>> net/openvswitch/flow.c | 30 ++++++++++++++++++++++--------
>> 2 files changed, 22 insertions(+), 9 deletions(-)
> ...
> ...
>> @@ -531,15 +538,22 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>> if (unlikely(parse_vlan(skb, key)))
>> return -ENOMEM;
>>
>> - skb->protocol = parse_ethertype(skb);
>> - if (unlikely(skb->protocol == htons(0)))
>> + key->eth.type = parse_ethertype(skb);
>> + if (unlikely(key->eth.type == htons(0)))
>> return -ENOMEM;
>>
>> + if (skb->protocol == htons(ETH_P_TEB)) {
>> + if (key->eth.vlan.tci & htons(VLAN_TAG_PRESENT)
>> + && !skb_vlan_tag_present(skb))
>> + skb->protocol = key->eth.vlan.tpid;
>> + else
>> + skb->protocol = key->eth.type;
>> + }
>> +
>
> I am not sure if this work in case of nested vlans.
> Can we move skb-protocol assignment to parse_vlan() to avoid checking
> for non-accelerated vlan case again here?
I did this for the v3 that I sent out a moment ago.
Thanks for the review,
Jarno
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-29 23:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-29 2:41 [PATCH v2 net-next 1/2] openvswitch: Add a missing break statement Jarno Rajahalme
2016-11-29 2:41 ` [PATCH v2 net-next 2/2] openvswitch: Fix skb->protocol for vlan frames Jarno Rajahalme
2016-11-29 7:21 ` Pravin Shelar
2016-11-29 23:32 ` Jarno Rajahalme
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.