Pravin, please look at comment inline below:
On 10/27/15 1:22 PM, Pravin Shelar wrote:
On Tue, Oct 27, 2015 at 9:45 AM, Thomas F Herbert
<thomasfherb...@gmail.com> wrote:
On 10/26/15 10:10 PM, Pravin Shelar wrote:
Thanks for the review.
On Sun, Oct 25, 2015 at 5:11 PM, Thomas F Herbert
<thomasfherb...@gmail.com> wrote:
Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.
Outer
TPID is also encoded in the flow key.
Signed-off-by: Thomas F Herbert <thomasfherb...@gmail.com>
This patch does not apply on current master due to conflicts related
net-branch merge.
OK, I will rebase.
Pravin, I implemented all reviewer comments and completing rebasing to
latest net-next upstream. The patch works well and flows install
properly. However, there may be one remaining issue. The vport-dev
transmit function now calls dev_queue_transmit() directly because it is
registered as the vport send op. The affect of this is that the vlan mtu
adjustment code in ovs_netdev_send() that was patched for mtu adjustment
for single and double tagged vlans is gone.Could you please verify that
the size adjustment is no longer needed.
---
net/openvswitch/actions.c | 6 +-
net/openvswitch/flow.c | 76 ++++++++++++----
net/openvswitch/flow.h | 8 +-
net/openvswitch/flow_netlink.c | 199
+++++++++++++++++++++++++++++++++++++----
net/openvswitch/vport-netdev.c | 4 +-
5 files changed, 252 insertions(+), 41 deletions(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index c8db44a..ed19e2b 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -302,24 +302,68 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
sizeof(struct icmp6hdr));
}
-static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
+/* Parse vlan tag from vlan header.
+ * Returns ERROR on memory error.
+ * Returns 0 if it encounters a non-vlan or incomplete packet.
+ * Returns 1 after successfully parsing vlan tag.
+ */
+
+static int parse_vlan_tag(struct sk_buff *skb, struct vlan_head *vlan)
{
- struct qtag_prefix {
- __be16 eth_type; /* ETH_P_8021Q */
- __be16 tci;
- };
- struct qtag_prefix *qp;
+ struct vlan_head *qp = (struct vlan_head *)skb->data;
+
+ if (likely(!eth_type_vlan(qp->tpid)))
+ return 0;
- if (unlikely(skb->len < sizeof(struct qtag_prefix) +
sizeof(__be16)))
+ if (unlikely(skb->len < sizeof(struct vlan_head) +
sizeof(__be16)))
return 0;
Why do we need extra sizeof(__be16) bytes here?
I don't have an answer to your question. I didn't write this code and have
wondered about why the extra two bytes were reserved. I don't know why it
should be necessarily for inner or outer vlans or the HW accelerated case or
for the non-accelerated case. If no reviewer can state a case for it, I will
remove it with the next version of this patch.
Looks like it is optimization for parsing ethertype, So lets keep it.
} else if (!tci) {
/* Corner case for truncated 802.1Q header. */
if (nla_len(encap)) {
@@ -1169,7 +1312,7 @@ int ovs_nla_get_match(struct net *net, struct
sw_flow_match *match,
goto free_newmask;
/* Always match on tci. */
- SW_FLOW_KEY_PUT(match, eth.tci, htons(0xffff), true);
+ SW_FLOW_KEY_PUT(match, eth.vlan.tci, htons(0xffff),
true);
Also need to exact match on inner tci.
This code sets a match on tci even if no vlan is present. Is this is for the
case where there is no explicit mask specified in the netlink encoded flow?
If that is correct, then it does need to be done for the inner vlan too.
Yes, By default it needs to be matched. userspace can overwrite it
with different wildcard.
--
Thomas F Herbert Red Hat
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
