Netdev List
 help / color / mirror / Atom feed
* [patch net-next v3 0/9] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang

Jiri Pirko (9):
  openvswitch: actions: use skb_postpull_rcsum when possible
  vlan: make __vlan_hwaccel_put_tag return void
  vlan: kill vlan_put_tag helper
  vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
  vlan: introduce *vlan_hwaccel_push_inside helpers
  vlan: introduce __vlan_insert_tag helper which does not free skb
  net: move make_writable helper into common code
  net: move vlan pop/push functions into common code
  sched: introduce vlan action

 drivers/net/bonding/bond_alb.c              |  17 +--
 drivers/net/bonding/bond_main.c             |  12 +-
 drivers/net/ethernet/emulex/benet/be_main.c |   6 +-
 drivers/net/usb/cdc_mbim.c                  |   2 +-
 drivers/net/vxlan.c                         |  22 +--
 drivers/scsi/fcoe/fcoe.c                    |   6 +-
 include/linux/if_vlan.h                     | 107 +++++++++-----
 include/linux/skbuff.h                      |   3 +
 include/net/tc_act/tc_vlan.h                |  27 ++++
 include/uapi/linux/tc_act/tc_vlan.h         |  35 +++++
 net/8021q/vlan_dev.c                        |   2 +-
 net/bridge/br_vlan.c                        |   4 +-
 net/core/dev.c                              |   8 +-
 net/core/netpoll.c                          |   4 +-
 net/core/skbuff.c                           | 107 ++++++++++++++
 net/ipv4/geneve.c                           |  12 +-
 net/openvswitch/actions.c                   | 130 ++++-------------
 net/openvswitch/datapath.c                  |   5 +-
 net/openvswitch/vport-gre.c                 |  12 +-
 net/sched/Kconfig                           |  11 ++
 net/sched/Makefile                          |   1 +
 net/sched/act_vlan.c                        | 207 ++++++++++++++++++++++++++++
 22 files changed, 525 insertions(+), 215 deletions(-)
 create mode 100644 include/net/tc_act/tc_vlan.h
 create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
 create mode 100644 net/sched/act_vlan.c

-- 
1.9.3

^ permalink raw reply

* [patch net-next v3 1/9] openvswitch: actions: use skb_postpull_rcsum when possible
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

Replace duplicated code by calling skb_postpull_rcsum

Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/openvswitch/actions.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 394efa6..749a301 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -175,10 +175,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	if (unlikely(err))
 		return err;
 
-	if (skb->ip_summed == CHECKSUM_COMPLETE)
-		skb->csum = csum_sub(skb->csum,
-				     csum_partial(skb_mpls_header(skb),
-						  MPLS_HLEN, 0));
+	skb_postpull_rcsum(skb, skb_mpls_header(skb), MPLS_HLEN);
 
 	memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
 		skb->mac_len);
@@ -230,9 +227,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
 	if (unlikely(err))
 		return err;
 
-	if (skb->ip_summed == CHECKSUM_COMPLETE)
-		skb->csum = csum_sub(skb->csum, csum_partial(skb->data
-					+ (2 * ETH_ALEN), VLAN_HLEN, 0));
+	skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
 
 	vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
 	*current_tci = vhdr->h_vlan_TCI;
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 2/9] vlan: make __vlan_hwaccel_put_tag return void
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

Always returns the same skb it gets, so change to void.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/scsi/fcoe/fcoe.c | 6 ++----
 include/linux/if_vlan.h  | 9 ++++-----
 net/8021q/vlan_dev.c     | 2 +-
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 4a8ac7d..73a8cc4 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1669,10 +1669,8 @@ static int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 	    fcoe->realdev->features & NETIF_F_HW_VLAN_CTAG_TX) {
 		/* must set skb->dev before calling vlan_put_tag */
 		skb->dev = fcoe->realdev;
-		skb = __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
-					     vlan_dev_vlan_id(fcoe->netdev));
-		if (!skb)
-			return -ENOMEM;
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+				       vlan_dev_vlan_id(fcoe->netdev));
 	} else
 		skb->dev = fcoe->netdev;
 
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index d69f057..1b5dbc2 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -347,13 +347,11 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
  *
  * Puts the VLAN TCI in @skb->vlan_tci and lets the device do the rest
  */
-static inline struct sk_buff *__vlan_hwaccel_put_tag(struct sk_buff *skb,
-						     __be16 vlan_proto,
-						     u16 vlan_tci)
+static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
+					  __be16 vlan_proto, u16 vlan_tci)
 {
 	skb->vlan_proto = vlan_proto;
 	skb->vlan_tci = VLAN_TAG_PRESENT | vlan_tci;
-	return skb;
 }
 
 /**
@@ -368,7 +366,8 @@ static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
 					   __be16 vlan_proto, u16 vlan_tci)
 {
 	if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
-		return __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+		__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+		return skb;
 	} else {
 		return __vlan_put_tag(skb, vlan_proto, vlan_tci);
 	}
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 0d441ec..d6524b2 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -150,7 +150,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 		u16 vlan_tci;
 		vlan_tci = vlan->vlan_id;
 		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
-		skb = __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
+		__vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
 	}
 
 	skb->dev = vlan->real_dev;
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 3/9] vlan: kill vlan_put_tag helper
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

Since both tx and rx paths work with skb->vlan_tci, there's no need for
this function anymore. Switch users directly to __vlan_hwaccel_put_tag.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/bonding/bond_alb.c  | 17 ++++-------------
 drivers/net/bonding/bond_main.c |  8 ++------
 drivers/net/usb/cdc_mbim.c      |  2 +-
 include/linux/if_vlan.h         | 19 -------------------
 4 files changed, 7 insertions(+), 39 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e1f1a00..bb9e9fc 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -475,12 +475,8 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 		skb->dev = client_info->slave->dev;
 
 		if (client_info->vlan_id) {
-			skb = vlan_put_tag(skb, htons(ETH_P_8021Q), client_info->vlan_id);
-			if (!skb) {
-				netdev_err(client_info->slave->bond->dev,
-					   "failed to insert VLAN tag\n");
-				continue;
-			}
+			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+					       client_info->vlan_id);
 		}
 
 		arp_xmit(skb);
@@ -951,13 +947,8 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
 	skb->priority = TC_PRIO_CONTROL;
 	skb->dev = slave->dev;
 
-	if (vid) {
-		skb = vlan_put_tag(skb, vlan_proto, vid);
-		if (!skb) {
-			netdev_err(slave->bond->dev, "failed to insert VLAN tag\n");
-			return;
-		}
-	}
+	if (vid)
+		__vlan_hwaccel_put_tag(skb, vlan_proto, vid);
 
 	dev_queue_xmit(skb);
 }
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8575fee..e26c682 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2159,12 +2159,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
 	if (outer_tag->vlan_id) {
 		netdev_dbg(slave_dev, "outer tag: proto %X vid %X\n",
 			   ntohs(outer_tag->vlan_proto), outer_tag->vlan_id);
-		skb = vlan_put_tag(skb, outer_tag->vlan_proto,
-				   outer_tag->vlan_id);
-		if (!skb) {
-			net_err_ratelimited("failed to insert outer VLAN tag\n");
-			return;
-		}
+		__vlan_hwaccel_put_tag(skb, outer_tag->vlan_proto,
+				       outer_tag->vlan_id);
 	}
 
 xmit:
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 5ee7a1d..96fc8a5 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -402,7 +402,7 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
 
 	/* map MBIM session to VLAN */
 	if (tci)
-		vlan_put_tag(skb, htons(ETH_P_8021Q), tci);
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), tci);
 err:
 	return skb;
 }
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 1b5dbc2..75b70a5 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -355,25 +355,6 @@ static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
 }
 
 /**
- * vlan_put_tag - inserts VLAN tag according to device features
- * @skb: skbuff to tag
- * @vlan_tci: VLAN TCI to insert
- *
- * Assumes skb->dev is the target that will xmit this frame.
- * Returns a VLAN tagged skb.
- */
-static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
-					   __be16 vlan_proto, u16 vlan_tci)
-{
-	if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
-		__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
-		return skb;
-	} else {
-		return __vlan_put_tag(skb, vlan_proto, vlan_tci);
-	}
-}
-
-/**
  * __vlan_get_tag - get the VLAN ID that is part of the payload
  * @skb: skbuff to query
  * @vlan_tci: buffer to store vlaue
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 4/9] vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

Name fits better. Plus there's going to be introduced
__vlan_insert_tag later on.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/bonding/bond_main.c             |  4 ++--
 drivers/net/ethernet/emulex/benet/be_main.c |  6 ++++--
 drivers/net/vxlan.c                         | 12 ++++++------
 include/linux/if_vlan.h                     |  8 +++++---
 net/bridge/br_vlan.c                        |  4 ++--
 net/core/dev.c                              |  4 ++--
 net/core/netpoll.c                          |  4 ++--
 net/ipv4/geneve.c                           | 11 +++++------
 net/openvswitch/actions.c                   |  4 +++-
 net/openvswitch/datapath.c                  |  3 ++-
 net/openvswitch/vport-gre.c                 |  6 +++---
 11 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e26c682..c1d7da4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2146,8 +2146,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
 
 		netdev_dbg(slave_dev, "inner tag: proto %X vid %X\n",
 			   ntohs(outer_tag->vlan_proto), tags->vlan_id);
-		skb = __vlan_put_tag(skb, tags->vlan_proto,
-				     tags->vlan_id);
+		skb = vlan_insert_tag_set_proto(skb, tags->vlan_proto,
+						tags->vlan_id);
 		if (!skb) {
 			net_err_ratelimited("failed to insert inner VLAN tag\n");
 			return;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 54160cc..d02fbc7 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -887,7 +887,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
 	}
 
 	if (vlan_tag) {
-		skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
+		skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
+						vlan_tag);
 		if (unlikely(!skb))
 			return skb;
 		skb->vlan_tci = 0;
@@ -896,7 +897,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
 	/* Insert the outer VLAN, if any */
 	if (adapter->qnq_vid) {
 		vlan_tag = adapter->qnq_vid;
-		skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
+		skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
+						vlan_tag);
 		if (unlikely(!skb))
 			return skb;
 		if (skip_hw_vlan)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 23b1e8c..bb8fbab 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1600,9 +1600,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
 		return err;
 
 	if (vlan_tx_tag_present(skb)) {
-		if (WARN_ON(!__vlan_put_tag(skb,
-					    skb->vlan_proto,
-					    vlan_tx_tag_get(skb))))
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (WARN_ON(!skb))
 			return -ENOMEM;
 
 		skb->vlan_tci = 0;
@@ -1644,9 +1644,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
 		return err;
 
 	if (vlan_tx_tag_present(skb)) {
-		if (WARN_ON(!__vlan_put_tag(skb,
-					    skb->vlan_proto,
-					    vlan_tx_tag_get(skb))))
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (WARN_ON(!skb))
 			return -ENOMEM;
 
 		skb->vlan_tci = 0;
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 75b70a5..46e4a15 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -320,8 +320,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
 }
 
 /**
- * __vlan_put_tag - regular VLAN tag inserting
+ * vlan_insert_tag_set_proto - regular VLAN tag inserting
  * @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
  * @vlan_tci: VLAN TCI to insert
  *
  * Inserts the VLAN tag into @skb as part of the payload
@@ -330,8 +331,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
  * Following the skb_unshare() example, in case of error, the calling function
  * doesn't have to worry about freeing the original skb.
  */
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
-					     __be16 vlan_proto, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
+							__be16 vlan_proto,
+							u16 vlan_tci)
 {
 	skb = vlan_insert_tag(skb, vlan_proto, vlan_tci);
 	if (skb)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 150048f..97b8ddf 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -199,8 +199,8 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 		if (skb->vlan_proto != proto) {
 			/* Protocol-mismatch, empty out vlan_tci for new tag */
 			skb_push(skb, ETH_HLEN);
-			skb = __vlan_put_tag(skb, skb->vlan_proto,
-					     vlan_tx_tag_get(skb));
+			skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+							vlan_tx_tag_get(skb));
 			if (unlikely(!skb))
 				return false;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 1ab168e..3611e60 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2645,8 +2645,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
 {
 	if (vlan_tx_tag_present(skb) &&
 	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = __vlan_put_tag(skb, skb->vlan_proto,
-				     vlan_tx_tag_get(skb));
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
 		if (skb)
 			skb->vlan_tci = 0;
 	}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e6645b4..65d3723 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -79,8 +79,8 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	if (vlan_tx_tag_present(skb) &&
 	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = __vlan_put_tag(skb, skb->vlan_proto,
-				     vlan_tx_tag_get(skb));
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
 		if (unlikely(!skb)) {
 			/* This is actually a packet drop, but we
 			 * don't want the code that calls this
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index 31802af..fd430a6 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -132,12 +132,11 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 		return err;
 
 	if (vlan_tx_tag_present(skb)) {
-		if (unlikely(!__vlan_put_tag(skb,
-					     skb->vlan_proto,
-					     vlan_tx_tag_get(skb)))) {
-			err = -ENOMEM;
-			return err;
-		}
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (unlikely(!skb)
+			return -ENOMEM;
+
 		skb->vlan_tci = 0;
 	}
 
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 749a301..426b913 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -287,7 +287,9 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
 		/* push down current VLAN tag */
 		current_tag = vlan_tx_tag_get(skb);
 
-		if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						current_tag);
+		if (!skb)
 			return -ENOMEM;
 		/* Update mac_len for subsequent MPLS actions */
 		skb->mac_len += VLAN_HLEN;
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ab141d4..c63e60e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -425,7 +425,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 		if (!nskb)
 			return -ENOMEM;
 
-		nskb = __vlan_put_tag(nskb, nskb->vlan_proto, vlan_tx_tag_get(nskb));
+		nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
+						 vlan_tx_tag_get(nskb));
 		if (!nskb)
 			return -ENOMEM;
 
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 8e61a5c..777cd8c 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -176,9 +176,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 	}
 
 	if (vlan_tx_tag_present(skb)) {
-		if (unlikely(!__vlan_put_tag(skb,
-					     skb->vlan_proto,
-					     vlan_tx_tag_get(skb)))) {
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (unlikely(!skb) {
 			err = -ENOMEM;
 			goto err_free_rt;
 		}
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 5/9] vlan: introduce *vlan_hwaccel_push_inside helpers
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

Use them to push skb->vlan_tci into the payload and avoid code
duplication.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/vxlan.c         | 22 ++++++----------------
 include/linux/if_vlan.h     | 34 ++++++++++++++++++++++++++++++++++
 net/core/dev.c              |  8 ++------
 net/core/netpoll.c          |  4 +---
 net/ipv4/geneve.c           | 11 +++--------
 net/openvswitch/datapath.c  |  6 +-----
 net/openvswitch/vport-gre.c | 12 ++++--------
 7 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bb8fbab..64d45fa 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1599,14 +1599,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
 	if (unlikely(err))
 		return err;
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (WARN_ON(!skb))
-			return -ENOMEM;
-
-		skb->vlan_tci = 0;
-	}
+	skb = vlan_hwaccel_push_inside(skb);
+	if (WARN_ON(!skb))
+		return -ENOMEM;
 
 	vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
 	vxh->vx_flags = htonl(VXLAN_FLAGS);
@@ -1643,14 +1638,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
 	if (unlikely(err))
 		return err;
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (WARN_ON(!skb))
-			return -ENOMEM;
-
-		skb->vlan_tci = 0;
-	}
+	skb = vlan_hwaccel_push_inside(skb);
+	if (WARN_ON(!skb))
+		return -ENOMEM;
 
 	vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
 	vxh->vx_flags = htonl(VXLAN_FLAGS);
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 46e4a15..291e670 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -341,6 +341,40 @@ static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
 	return skb;
 }
 
+/*
+ * __vlan_hwaccel_push_inside - pushes vlan tag to the payload
+ * @skb: skbuff to tag
+ *
+ * Pushes the VLAN tag from @skb->vlan_tci inside to the payload.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb)
+{
+	skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+					vlan_tx_tag_get(skb));
+	if (likely(skb))
+		skb->vlan_tci = 0;
+	return skb;
+}
+/*
+ * vlan_hwaccel_push_inside - pushes vlan tag to the payload
+ * @skb: skbuff to tag
+ *
+ * Checks is tag is present in @skb->vlan_tci and if it is, it pushes the
+ * VLAN tag from @skb->vlan_tci inside to the payload.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *vlan_hwaccel_push_inside(struct sk_buff *skb)
+{
+	if (vlan_tx_tag_present(skb))
+		skb = __vlan_hwaccel_push_inside(skb);
+	return skb;
+}
+
 /**
  * __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
  * @skb: skbuff to tag
diff --git a/net/core/dev.c b/net/core/dev.c
index 3611e60..ac48362 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2644,12 +2644,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
 					  netdev_features_t features)
 {
 	if (vlan_tx_tag_present(skb) &&
-	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (skb)
-			skb->vlan_tci = 0;
-	}
+	    !vlan_hw_offload_capable(features, skb->vlan_proto))
+		skb = __vlan_hwaccel_push_inside(skb);
 	return skb;
 }
 
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 65d3723..e0ad5d1 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -79,8 +79,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	if (vlan_tx_tag_present(skb) &&
 	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
+		skb = __vlan_hwaccel_push_inside(skb);
 		if (unlikely(!skb)) {
 			/* This is actually a packet drop, but we
 			 * don't want the code that calls this
@@ -88,7 +87,6 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			 */
 			goto out;
 		}
-		skb->vlan_tci = 0;
 	}
 
 	status = netdev_start_xmit(skb, dev, txq, false);
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index fd430a6..a457232 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -131,14 +131,9 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 	if (unlikely(err))
 		return err;
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (unlikely(!skb)
-			return -ENOMEM;
-
-		skb->vlan_tci = 0;
-	}
+	skb = vlan_hwaccel_push_inside(skb);
+	if (unlikely(!skb))
+		return -ENOMEM;
 
 	gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);
 	geneve_build_header(gnvh, tun_flags, vni, opt_len, opt);
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index c63e60e..0cb9d4f 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -425,12 +425,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 		if (!nskb)
 			return -ENOMEM;
 
-		nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
-						 vlan_tx_tag_get(nskb));
-		if (!nskb)
-			return -ENOMEM;
+		nskb = __vlan_hwaccel_push_inside(nskb);
 
-		nskb->vlan_tci = 0;
 		skb = nskb;
 	}
 
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 777cd8c..6b69df5 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -175,14 +175,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 			goto err_free_rt;
 	}
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (unlikely(!skb) {
-			err = -ENOMEM;
-			goto err_free_rt;
-		}
-		skb->vlan_tci = 0;
+	skb = vlan_hwaccel_push_inside(skb);
+	if (unlikely(!skb)) {
+		err = -ENOMEM;
+		goto err_free_rt;
 	}
 
 	/* Push Tunnel header. */
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 6/9] vlan: introduce __vlan_insert_tag helper which does not free skb
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

There's a need for helper which inserts vlan tag but does not free the
skb in case of an error.

Suggested-by: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/if_vlan.h | 45 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 291e670..515a35e 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -282,28 +282,24 @@ static inline bool vlan_hw_offload_capable(netdev_features_t features,
 }
 
 /**
- * vlan_insert_tag - regular VLAN tag inserting
+ * __vlan_insert_tag - regular VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_proto: VLAN encapsulation protocol
  * @vlan_tci: VLAN TCI to insert
  *
  * Inserts the VLAN tag into @skb as part of the payload
- * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
- *
- * Following the skb_unshare() example, in case of error, the calling function
- * doesn't have to worry about freeing the original skb.
+ * Returns error if skb_cow_head failes.
  *
  * Does not change skb->protocol so this function can be used during receive.
  */
-static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
-					      __be16 vlan_proto, u16 vlan_tci)
+static inline int __vlan_insert_tag(struct sk_buff *skb,
+				    __be16 vlan_proto, u16 vlan_tci)
 {
 	struct vlan_ethhdr *veth;
 
-	if (skb_cow_head(skb, VLAN_HLEN) < 0) {
-		dev_kfree_skb_any(skb);
-		return NULL;
-	}
+	if (skb_cow_head(skb, VLAN_HLEN) < 0)
+		return -ENOMEM;
+
 	veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN);
 
 	/* Move the mac addresses to the beginning of the new header. */
@@ -316,6 +312,33 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
 	/* now, the TCI */
 	veth->h_vlan_TCI = htons(vlan_tci);
 
+	return 0;
+}
+
+/**
+ * vlan_insert_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
+ * @vlan_tci: VLAN TCI to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ *
+ * Does not change skb->protocol so this function can be used during receive.
+ */
+static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
+					      __be16 vlan_proto, u16 vlan_tci)
+{
+	int err;
+
+	err = __vlan_insert_tag(skb, vlan_proto, vlan_tci);
+	if (err) {
+		dev_kfree_skb_any(skb);
+		return NULL;
+	}
 	return skb;
 }
 
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 7/9] net: move make_writable helper into common code
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

note that skb_make_writable already exists in net/netfilter/core.c
but does something slightly different.

Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 include/linux/skbuff.h    |  1 +
 net/core/skbuff.c         | 12 ++++++++++++
 net/openvswitch/actions.c | 39 ++++++++++++++-------------------------
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 73c370e..e045516 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2678,6 +2678,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
 unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
 struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
+int skb_ensure_writable(struct sk_buff *skb, int write_len);
 
 struct skb_checksum_ops {
 	__wsum (*update)(const void *mem, int len, __wsum wsum);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7001896..d11bbe0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4151,6 +4151,18 @@ err_free:
 }
 EXPORT_SYMBOL(skb_vlan_untag);
 
+int skb_ensure_writable(struct sk_buff *skb, int write_len)
+{
+	if (!pskb_may_pull(skb, write_len))
+		return -ENOMEM;
+
+	if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
+		return 0;
+
+	return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+}
+EXPORT_SYMBOL(skb_ensure_writable);
+
 /**
  * alloc_skb_with_frags - allocate skb with page frags
  *
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 426b913..7ffa377 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -119,17 +119,6 @@ static bool is_flow_key_valid(const struct sw_flow_key *key)
 	return !!key->eth.type;
 }
 
-static int make_writable(struct sk_buff *skb, int write_len)
-{
-	if (!pskb_may_pull(skb, write_len))
-		return -ENOMEM;
-
-	if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
-		return 0;
-
-	return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
-}
-
 static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 		     const struct ovs_action_push_mpls *mpls)
 {
@@ -171,7 +160,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	struct ethhdr *hdr;
 	int err;
 
-	err = make_writable(skb, skb->mac_len + MPLS_HLEN);
+	err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -201,7 +190,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	__be32 *stack;
 	int err;
 
-	err = make_writable(skb, skb->mac_len + MPLS_HLEN);
+	err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -223,7 +212,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
 	struct vlan_hdr *vhdr;
 	int err;
 
-	err = make_writable(skb, VLAN_ETH_HLEN);
+	err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -310,7 +299,7 @@ static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
 			const struct ovs_key_ethernet *eth_key)
 {
 	int err;
-	err = make_writable(skb, ETH_HLEN);
+	err = skb_ensure_writable(skb, ETH_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -412,8 +401,8 @@ static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *key,
 	struct iphdr *nh;
 	int err;
 
-	err = make_writable(skb, skb_network_offset(skb) +
-				 sizeof(struct iphdr));
+	err = skb_ensure_writable(skb, skb_network_offset(skb) +
+				  sizeof(struct iphdr));
 	if (unlikely(err))
 		return err;
 
@@ -450,8 +439,8 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
 	__be32 *saddr;
 	__be32 *daddr;
 
-	err = make_writable(skb, skb_network_offset(skb) +
-			    sizeof(struct ipv6hdr));
+	err = skb_ensure_writable(skb, skb_network_offset(skb) +
+				  sizeof(struct ipv6hdr));
 	if (unlikely(err))
 		return err;
 
@@ -493,7 +482,7 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
 	return 0;
 }
 
-/* Must follow make_writable() since that can move the skb data. */
+/* Must follow skb_ensure_writable() since that can move the skb data. */
 static void set_tp_port(struct sk_buff *skb, __be16 *port,
 			 __be16 new_port, __sum16 *check)
 {
@@ -523,8 +512,8 @@ static int set_udp(struct sk_buff *skb, struct sw_flow_key *key,
 	struct udphdr *uh;
 	int err;
 
-	err = make_writable(skb, skb_transport_offset(skb) +
-				 sizeof(struct udphdr));
+	err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				  sizeof(struct udphdr));
 	if (unlikely(err))
 		return err;
 
@@ -548,8 +537,8 @@ static int set_tcp(struct sk_buff *skb, struct sw_flow_key *key,
 	struct tcphdr *th;
 	int err;
 
-	err = make_writable(skb, skb_transport_offset(skb) +
-				 sizeof(struct tcphdr));
+	err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				  sizeof(struct tcphdr));
 	if (unlikely(err))
 		return err;
 
@@ -574,7 +563,7 @@ static int set_sctp(struct sk_buff *skb, struct sw_flow_key *key,
 	int err;
 	unsigned int sctphoff = skb_transport_offset(skb);
 
-	err = make_writable(skb, sctphoff + sizeof(struct sctphdr));
+	err = skb_ensure_writable(skb, sctphoff + sizeof(struct sctphdr));
 	if (unlikely(err))
 		return err;
 
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 8/9] net: move vlan pop/push functions into common code
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

So it can be used from out of openvswitch code.
Did couple of cosmetic changes on the way, namely variable naming and
adding support for 8021AD proto.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---

v2->v3
- used previously introduced __vlan_insert_tag helper
- used skb_push/pull to get skb->data into needed point
- fixed skb->mac_len computation in skb_vlan_push pointed out by Pravin
v1->v2:
- adjusted to fix recent ovs changes
- included change to use make_writable suggested by Eric

 include/linux/skbuff.h    |  2 +
 net/core/skbuff.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 net/openvswitch/actions.c | 88 ++++++-------------------------------------
 3 files changed, 109 insertions(+), 76 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e045516..78c299f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2679,6 +2679,8 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
 struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
 int skb_ensure_writable(struct sk_buff *skb, int write_len);
+int skb_vlan_pop(struct sk_buff *skb);
+int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
 
 struct skb_checksum_ops {
 	__wsum (*update)(const void *mem, int len, __wsum wsum);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d11bbe0..2f72e62 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4163,6 +4163,101 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len)
 }
 EXPORT_SYMBOL(skb_ensure_writable);
 
+/* remove VLAN header from packet and update csum accordingly. */
+static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
+{
+	struct vlan_hdr *vhdr;
+	unsigned int offset = skb->data - skb_mac_header(skb);
+	int err;
+
+	__skb_push(skb, offset);
+	err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
+	if (unlikely(err))
+		goto pull;
+
+	skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
+
+	vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
+	*vlan_tci = ntohs(vhdr->h_vlan_TCI);
+
+	memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
+	__skb_pull(skb, VLAN_HLEN);
+
+	vlan_set_encap_proto(skb, vhdr);
+	skb->mac_header += VLAN_HLEN;
+
+	if (skb_network_offset(skb) < ETH_HLEN)
+		skb_set_network_header(skb, ETH_HLEN);
+
+	skb_reset_mac_len(skb);
+pull:
+	__skb_pull(skb, offset);
+
+	return err;
+}
+
+int skb_vlan_pop(struct sk_buff *skb)
+{
+	u16 vlan_tci;
+	__be16 vlan_proto;
+	int err;
+
+	if (likely(vlan_tx_tag_present(skb))) {
+		skb->vlan_tci = 0;
+	} else {
+		if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
+			      skb->protocol != htons(ETH_P_8021AD)) ||
+			     skb->len < VLAN_ETH_HLEN))
+			return 0;
+
+		err = __skb_vlan_pop(skb, &vlan_tci);
+		if (err)
+			return err;
+	}
+	/* move next vlan tag to hw accel tag */
+	if (likely((skb->protocol != htons(ETH_P_8021Q) &&
+		    skb->protocol != htons(ETH_P_8021AD)) ||
+		   skb->len < VLAN_ETH_HLEN))
+		return 0;
+
+	vlan_proto = skb->protocol;
+	err = __skb_vlan_pop(skb, &vlan_tci);
+	if (unlikely(err))
+		return err;
+
+	__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+	return 0;
+}
+EXPORT_SYMBOL(skb_vlan_pop);
+
+int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
+{
+	if (vlan_tx_tag_present(skb)) {
+		unsigned int offset = skb->data - skb_mac_header(skb);
+		int err;
+
+		/* __vlan_insert_tag expect skb->data pointing to mac header.
+		 * So change skb->data before calling it and change back to
+		 * original position later
+		 */
+		__skb_push(skb, offset);
+		err = __vlan_insert_tag(skb, skb->vlan_proto,
+					vlan_tx_tag_get(skb));
+		if (err)
+			return err;
+		skb->protocol = skb->vlan_proto;
+		skb_reset_mac_len(skb);
+		__skb_pull(skb, offset);
+
+		if (skb->ip_summed == CHECKSUM_COMPLETE)
+			skb->csum = csum_add(skb->csum, csum_partial(skb->data
+					+ (2 * ETH_ALEN), VLAN_HLEN, 0));
+	}
+	__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+	return 0;
+}
+EXPORT_SYMBOL(skb_vlan_push);
+
 /**
  * alloc_skb_with_frags - allocate skb with page frags
  *
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 7ffa377..ae7e1b2 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -206,93 +206,27 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	return 0;
 }
 
-/* remove VLAN header from packet and update csum accordingly. */
-static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
-{
-	struct vlan_hdr *vhdr;
-	int err;
-
-	err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
-	if (unlikely(err))
-		return err;
-
-	skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
-
-	vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
-	*current_tci = vhdr->h_vlan_TCI;
-
-	memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
-	__skb_pull(skb, VLAN_HLEN);
-
-	vlan_set_encap_proto(skb, vhdr);
-	skb->mac_header += VLAN_HLEN;
-
-	if (skb_network_offset(skb) < ETH_HLEN)
-		skb_set_network_header(skb, ETH_HLEN);
-
-	/* Update mac_len for subsequent MPLS actions */
-	skb_reset_mac_len(skb);
-	return 0;
-}
-
 static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 {
-	__be16 tci;
 	int err;
 
-	if (likely(vlan_tx_tag_present(skb))) {
-		skb->vlan_tci = 0;
-	} else {
-		if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
-			     skb->len < VLAN_ETH_HLEN))
-			return 0;
-
-		err = __pop_vlan_tci(skb, &tci);
-		if (err)
-			return err;
-	}
-	/* move next vlan tag to hw accel tag */
-	if (likely(skb->protocol != htons(ETH_P_8021Q) ||
-		   skb->len < VLAN_ETH_HLEN)) {
+	err = skb_vlan_pop(skb);
+	if (vlan_tx_tag_present(skb))
+		invalidate_flow_key(key);
+	else
 		key->eth.tci = 0;
-		return 0;
-	}
-
-	invalidate_flow_key(key);
-	err = __pop_vlan_tci(skb, &tci);
-	if (unlikely(err))
-		return err;
-
-	__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
-	return 0;
+	return err;
 }
 
 static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
 		     const struct ovs_action_push_vlan *vlan)
 {
-	if (unlikely(vlan_tx_tag_present(skb))) {
-		u16 current_tag;
-
-		/* push down current VLAN tag */
-		current_tag = vlan_tx_tag_get(skb);
-
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						current_tag);
-		if (!skb)
-			return -ENOMEM;
-		/* Update mac_len for subsequent MPLS actions */
-		skb->mac_len += VLAN_HLEN;
-
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			skb->csum = csum_add(skb->csum, csum_partial(skb->data
-					+ (2 * ETH_ALEN), VLAN_HLEN, 0));
-
+	if (vlan_tx_tag_present(skb))
 		invalidate_flow_key(key);
-	} else {
+	else
 		key->eth.tci = vlan->vlan_tci;
-	}
-	__vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
-	return 0;
+	return skb_vlan_push(skb, vlan->vlan_tpid,
+			     ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
 }
 
 static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
@@ -858,8 +792,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 
 		case OVS_ACTION_ATTR_PUSH_VLAN:
 			err = push_vlan(skb, key, nla_data(a));
-			if (unlikely(err)) /* skb already freed. */
+			if (unlikely(err)) {
+				dev_kfree_skb_any(skb);
 				return err;
+			}
 			break;
 
 		case OVS_ACTION_ATTR_POP_VLAN:
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v3 9/9] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-18 21:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

This tc action allows to work with vlan tagged skbs. Two supported
sub-actions are header pop and header push.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---

v1->v2:
- fixed protocol dumping in tcf_vlan_dump

 include/net/tc_act/tc_vlan.h        |  27 +++++
 include/uapi/linux/tc_act/tc_vlan.h |  35 ++++++
 net/sched/Kconfig                   |  11 ++
 net/sched/Makefile                  |   1 +
 net/sched/act_vlan.c                | 207 ++++++++++++++++++++++++++++++++++++
 5 files changed, 281 insertions(+)
 create mode 100644 include/net/tc_act/tc_vlan.h
 create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
 create mode 100644 net/sched/act_vlan.c

diff --git a/include/net/tc_act/tc_vlan.h b/include/net/tc_act/tc_vlan.h
new file mode 100644
index 0000000..c809c1d
--- /dev/null
+++ b/include/net/tc_act/tc_vlan.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __NET_TC_VLAN_H
+#define __NET_TC_VLAN_H
+
+#include <net/act_api.h>
+
+#define VLAN_F_POP		0x1
+#define VLAN_F_PUSH		0x2
+
+struct tcf_vlan {
+	struct tcf_common	common;
+	int			tcfv_action;
+	__be16			tcfv_push_vid;
+	__be16			tcfv_push_proto;
+};
+#define to_vlan(a) \
+	container_of(a->priv, struct tcf_vlan, common)
+
+#endif /* __NET_TC_VLAN_H */
diff --git a/include/uapi/linux/tc_act/tc_vlan.h b/include/uapi/linux/tc_act/tc_vlan.h
new file mode 100644
index 0000000..f7b8d44
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_vlan.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __LINUX_TC_VLAN_H
+#define __LINUX_TC_VLAN_H
+
+#include <linux/pkt_cls.h>
+
+#define TCA_ACT_VLAN 12
+
+#define TCA_VLAN_ACT_POP	1
+#define TCA_VLAN_ACT_PUSH	2
+
+struct tc_vlan {
+	tc_gen;
+	int v_action;
+};
+
+enum {
+	TCA_VLAN_UNSPEC,
+	TCA_VLAN_TM,
+	TCA_VLAN_PARMS,
+	TCA_VLAN_PUSH_VLAN_ID,
+	TCA_VLAN_PUSH_VLAN_PROTOCOL,
+	__TCA_VLAN_MAX,
+};
+#define TCA_VLAN_MAX (__TCA_VLAN_MAX - 1)
+
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index a1a8e29..88618f8 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -686,6 +686,17 @@ config NET_ACT_CSUM
 	  To compile this code as a module, choose M here: the
 	  module will be called act_csum.
 
+config NET_ACT_VLAN
+        tristate "Vlan manipulation"
+        depends on NET_CLS_ACT
+        ---help---
+	  Say Y here to push or pop vlan headers.
+
+	  If unsure, say N.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called act_vlan.
+
 config NET_CLS_IND
 	bool "Incoming device classification"
 	depends on NET_CLS_U32 || NET_CLS_FW
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 0a869a1..679f24a 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_NET_ACT_PEDIT)	+= act_pedit.o
 obj-$(CONFIG_NET_ACT_SIMP)	+= act_simple.o
 obj-$(CONFIG_NET_ACT_SKBEDIT)	+= act_skbedit.o
 obj-$(CONFIG_NET_ACT_CSUM)	+= act_csum.o
+obj-$(CONFIG_NET_ACT_VLAN)	+= act_vlan.o
 obj-$(CONFIG_NET_SCH_FIFO)	+= sch_fifo.o
 obj-$(CONFIG_NET_SCH_CBQ)	+= sch_cbq.o
 obj-$(CONFIG_NET_SCH_HTB)	+= sch_htb.o
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
new file mode 100644
index 0000000..d735ecf
--- /dev/null
+++ b/net/sched/act_vlan.c
@@ -0,0 +1,207 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/if_vlan.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+
+#include <linux/tc_act/tc_vlan.h>
+#include <net/tc_act/tc_vlan.h>
+
+#define VLAN_TAB_MASK     15
+
+static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
+		    struct tcf_result *res)
+{
+	struct tcf_vlan *v = a->priv;
+	int action;
+	int err;
+
+	spin_lock(&v->tcf_lock);
+	v->tcf_tm.lastuse = jiffies;
+	bstats_update(&v->tcf_bstats, skb);
+	action = v->tcf_action;
+
+	switch (v->tcfv_action) {
+	case TCA_VLAN_ACT_POP:
+		err = skb_vlan_pop(skb);
+		if (err)
+			goto drop;
+		break;
+	case TCA_VLAN_ACT_PUSH:
+		err = skb_vlan_push(skb, v->tcfv_push_proto, v->tcfv_push_vid);
+		if (err)
+			goto drop;
+		break;
+	default:
+		BUG();
+	}
+
+	goto unlock;
+
+drop:
+	action = TC_ACT_SHOT;
+	v->tcf_qstats.drops++;
+unlock:
+	spin_unlock(&v->tcf_lock);
+	return action;
+}
+
+static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = {
+	[TCA_VLAN_PARMS]		= { .len = sizeof(struct tc_vlan) },
+	[TCA_VLAN_PUSH_VLAN_ID]		= { .type = NLA_U16 },
+	[TCA_VLAN_PUSH_VLAN_PROTOCOL]	= { .type = NLA_U16 },
+};
+
+static int tcf_vlan_init(struct net *net, struct nlattr *nla,
+			 struct nlattr *est, struct tc_action *a,
+			 int ovr, int bind)
+{
+	struct nlattr *tb[TCA_VLAN_MAX + 1];
+	struct tc_vlan *parm;
+	struct tcf_vlan *v;
+	int action;
+	__be16 push_vid = 0;
+	__be16 push_proto = 0;
+	int ret = 0;
+	int err;
+
+	if (!nla)
+		return -EINVAL;
+
+	err = nla_parse_nested(tb, TCA_VLAN_MAX, nla, vlan_policy);
+	if (err < 0)
+		return err;
+
+	if (!tb[TCA_VLAN_PARMS])
+		return -EINVAL;
+	parm = nla_data(tb[TCA_VLAN_PARMS]);
+	switch (parm->v_action) {
+	case TCA_VLAN_ACT_POP:
+		break;
+	case TCA_VLAN_ACT_PUSH:
+		if (!tb[TCA_VLAN_PUSH_VLAN_ID])
+			return -EINVAL;
+		push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
+		if (push_vid >= VLAN_VID_MASK)
+			return -ERANGE;
+
+		if (tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]) {
+			push_proto = nla_get_be16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]);
+			switch (push_proto) {
+			case htons(ETH_P_8021Q):
+			case htons(ETH_P_8021AD):
+				break;
+			default:
+				return -EPROTONOSUPPORT;
+			}
+		} else {
+			push_proto = htons(ETH_P_8021Q);
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+	action = parm->v_action;
+
+	if (!tcf_hash_check(parm->index, a, bind)) {
+		ret = tcf_hash_create(parm->index, est, a, sizeof(*v), bind);
+		if (ret)
+			return ret;
+
+		ret = ACT_P_CREATED;
+	} else {
+		if (bind)
+			return 0;
+		tcf_hash_release(a, bind);
+		if (!ovr)
+			return -EEXIST;
+	}
+
+	v = to_vlan(a);
+
+	spin_lock_bh(&v->tcf_lock);
+
+	v->tcfv_action = action;
+	v->tcfv_push_vid = push_vid;
+	v->tcfv_push_proto = push_proto;
+
+	v->tcf_action = parm->action;
+
+	spin_unlock_bh(&v->tcf_lock);
+
+	if (ret == ACT_P_CREATED)
+		tcf_hash_insert(a);
+	return ret;
+}
+
+static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
+			 int bind, int ref)
+{
+	unsigned char *b = skb_tail_pointer(skb);
+	struct tcf_vlan *v = a->priv;
+	struct tc_vlan opt = {
+		.index    = v->tcf_index,
+		.refcnt   = v->tcf_refcnt - ref,
+		.bindcnt  = v->tcf_bindcnt - bind,
+		.action   = v->tcf_action,
+		.v_action = v->tcfv_action,
+	};
+	struct tcf_t t;
+
+	if (nla_put(skb, TCA_VLAN_PARMS, sizeof(opt), &opt))
+		goto nla_put_failure;
+
+	if (v->tcfv_action == TCA_VLAN_ACT_PUSH &&
+	    (nla_put_u16(skb, TCA_VLAN_PUSH_VLAN_ID, v->tcfv_push_vid) ||
+	     nla_put_be16(skb, TCA_VLAN_PUSH_VLAN_PROTOCOL, v->tcfv_push_proto)))
+		goto nla_put_failure;
+
+	t.install = jiffies_to_clock_t(jiffies - v->tcf_tm.install);
+	t.lastuse = jiffies_to_clock_t(jiffies - v->tcf_tm.lastuse);
+	t.expires = jiffies_to_clock_t(v->tcf_tm.expires);
+	if (nla_put(skb, TCA_VLAN_TM, sizeof(t), &t))
+		goto nla_put_failure;
+	return skb->len;
+
+nla_put_failure:
+	nlmsg_trim(skb, b);
+	return -1;
+}
+
+static struct tc_action_ops act_vlan_ops = {
+	.kind		=	"vlan",
+	.type		=	TCA_ACT_VLAN,
+	.owner		=	THIS_MODULE,
+	.act		=	tcf_vlan,
+	.dump		=	tcf_vlan_dump,
+	.init		=	tcf_vlan_init,
+};
+
+static int __init vlan_init_module(void)
+{
+	return tcf_register_action(&act_vlan_ops, VLAN_TAB_MASK);
+}
+
+static void __exit vlan_cleanup_module(void)
+{
+	tcf_unregister_action(&act_vlan_ops);
+}
+
+module_init(vlan_init_module);
+module_exit(vlan_cleanup_module);
+
+MODULE_AUTHOR("Jiri Pirko <jiri@resnulli.us>");
+MODULE_DESCRIPTION("vlan manipulation actions");
+MODULE_LICENSE("GPL v2");
-- 
1.9.3

^ permalink raw reply related

* Re: [RFC] situation with csum_and_copy_... API
From: Linus Torvalds @ 2014-11-18 21:39 UTC (permalink / raw)
  To: Al Viro; +Cc: Network Development, David Miller, Linux Kernel Mailing List
In-Reply-To: <20141118212307.GU7996@ZenIV.linux.org.uk>

On Tue, Nov 18, 2014 at 1:23 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> OK...  If netdev folks can live with that for now, I've no problem with
> dropping 3/5.  However, I really think we need a variant of csum-and-copy
> that would _not_ bother with access_ok() longer term.  That can wait, though...

iirc,access_ok() ends up being something like three or four
instructions. It's generally not very painful.

The main reason to ever use the "__" functions is for __get_user() or
__put_user() in a loop (or when doing multiple ones when
loading/storing a struct or a signal stack frame or whatever), and
then the real issue ends up being that the __get_user() is really
often just a single instruction, while "get_user()" is a function call
that does that access_ok().

So then moving the access_ok() outside the loop, or to above the
structure load, can make a *big* deal.  But removing the access_ok()
entirely tends to not be a huge issue - it's just that you want to do
it *once*, instead of doing it over-and-over.

There might be some case you really want to remove it from the
function entirely so that the "access_ok()" is no longer close to the
accesses it checks, but I really think you want to have a goof
performance case for it, and a comment about it.

And no, we haven't really always followed those rules.

Btw, these days, on x86, we actually have a bigger issue with the
whole STAC/CLAC thing:  it's a good safety measure, but doing
STAC/CLAC for each access is painful. So "__get_user()" and
"__put_user()" sadly aren't the single-instruction things they used to
be any more. The code sequences that really want tight code (think the
'struct stat' copying and the like) sadly cannot get it as it is..

                   Linus

^ permalink raw reply

* [patch iproute2 v3] tc: add support for vlan tc action
From: Jiri Pirko @ 2014-11-18 21:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Reviewed-by: Cong Wang <cwang@twopensource.com>
---

v1->v2:
- included changes suggested by Jamal

 include/linux/tc_act/tc_vlan.h |  35 +++++++
 tc/Makefile                    |   1 +
 tc/m_vlan.c                    | 221 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 257 insertions(+)
 create mode 100644 include/linux/tc_act/tc_vlan.h
 create mode 100644 tc/m_vlan.c

diff --git a/include/linux/tc_act/tc_vlan.h b/include/linux/tc_act/tc_vlan.h
new file mode 100644
index 0000000..f7b8d44
--- /dev/null
+++ b/include/linux/tc_act/tc_vlan.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __LINUX_TC_VLAN_H
+#define __LINUX_TC_VLAN_H
+
+#include <linux/pkt_cls.h>
+
+#define TCA_ACT_VLAN 12
+
+#define TCA_VLAN_ACT_POP	1
+#define TCA_VLAN_ACT_PUSH	2
+
+struct tc_vlan {
+	tc_gen;
+	int v_action;
+};
+
+enum {
+	TCA_VLAN_UNSPEC,
+	TCA_VLAN_TM,
+	TCA_VLAN_PARMS,
+	TCA_VLAN_PUSH_VLAN_ID,
+	TCA_VLAN_PUSH_VLAN_PROTOCOL,
+	__TCA_VLAN_MAX,
+};
+#define TCA_VLAN_MAX (__TCA_VLAN_MAX - 1)
+
+#endif
diff --git a/tc/Makefile b/tc/Makefile
index 1ab36c6..830c97d 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -40,6 +40,7 @@ TCMODULES += m_pedit.o
 TCMODULES += m_skbedit.o
 TCMODULES += m_csum.o
 TCMODULES += m_simple.o
+TCMODULES += m_vlan.o
 TCMODULES += p_ip.o
 TCMODULES += p_icmp.o
 TCMODULES += p_tcp.o
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
new file mode 100644
index 0000000..872bf72
--- /dev/null
+++ b/tc/m_vlan.c
@@ -0,0 +1,221 @@
+/*
+ * m_vlan.c		vlan manipulation module
+ *
+ *              This program is free software; you can redistribute it and/or
+ *              modify it under the terms of the GNU General Public License
+ *              as published by the Free Software Foundation; either version
+ *              2 of the License, or (at your option) any later version.
+ *
+ * Authors:     Jiri Pirko <jiri@resnulli.us>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <linux/if_ether.h>
+#include "utils.h"
+#include "rt_names.h"
+#include "tc_util.h"
+#include <linux/tc_act/tc_vlan.h>
+
+static void explain(void)
+{
+	fprintf(stderr, "Usage: vlan pop\n");
+	fprintf(stderr, "       vlan push [ protocol VLANPROTO ] id VLANID\n");
+	fprintf(stderr, "       VLANPROTO is one of 802.1Q or 802.1AD\n");
+	fprintf(stderr, "            with default: 802.1Q\n");
+}
+
+static void usage(void)
+{
+	explain();
+	exit(-1);
+}
+
+static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
+		      int tca_id, struct nlmsghdr *n)
+{
+	int argc = *argc_p;
+	char **argv = *argv_p;
+	struct rtattr *tail;
+	int action = 0;
+	__u16 id;
+	int id_set = 0;
+	__u16 proto;
+	int proto_set = 0;
+	struct tc_vlan parm = { 0 };
+
+	if (matches(*argv, "vlan") != 0)
+		return -1;
+
+	NEXT_ARG();
+
+	while (argc > 0) {
+		if (matches(*argv, "pop") == 0) {
+			if (action) {
+				fprintf(stderr, "unexpexted \"%s\" - action already specified\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			action = TCA_VLAN_ACT_POP;
+		} else if (matches(*argv, "push") == 0) {
+			if (action) {
+				fprintf(stderr, "unexpexted \"%s\" - action already specified\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			action = TCA_VLAN_ACT_PUSH;
+		} else if (matches(*argv, "id") == 0) {
+			if (action != TCA_VLAN_ACT_PUSH) {
+				fprintf(stderr, "\"%s\" is only valid for push\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			NEXT_ARG();
+			if (get_u16(&id, *argv, 0))
+				invarg("id is invalid", *argv);
+			id_set = 1;
+		} else if (matches(*argv, "protocol") == 0) {
+			if (action != TCA_VLAN_ACT_PUSH) {
+				fprintf(stderr, "\"%s\" is only valid for push\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			NEXT_ARG();
+			if (ll_proto_a2n(&proto, *argv))
+				invarg("protocol is invalid", *argv);
+			proto_set = 1;
+		} else if (matches(*argv, "help") == 0) {
+			usage();
+		} else {
+			break;
+		}
+		argc--;
+		argv++;
+	}
+
+	parm.action = TC_ACT_PIPE;
+	if (argc) {
+		if (matches(*argv, "reclassify") == 0) {
+			parm.action = TC_ACT_RECLASSIFY;
+			NEXT_ARG();
+		} else if (matches(*argv, "pipe") == 0) {
+			parm.action = TC_ACT_PIPE;
+			NEXT_ARG();
+		} else if (matches(*argv, "drop") == 0 ||
+			   matches(*argv, "shot") == 0) {
+			parm.action = TC_ACT_SHOT;
+			NEXT_ARG();
+		} else if (matches(*argv, "continue") == 0) {
+			parm.action = TC_ACT_UNSPEC;
+			NEXT_ARG();
+		} else if (matches(*argv, "pass") == 0) {
+			parm.action = TC_ACT_OK;
+			NEXT_ARG();
+		}
+	}
+
+	if (argc) {
+		if (matches(*argv, "index") == 0) {
+			NEXT_ARG();
+			if (get_u32(&parm.index, *argv, 10)) {
+				fprintf(stderr, "vlan: Illegal \"index\"\n");
+				return -1;
+			}
+			argc--;
+			argv++;
+		}
+	}
+
+	if (action == TCA_VLAN_ACT_PUSH && !id_set) {
+		fprintf(stderr, "id needs to be set for push\n");
+		explain();
+		return -1;
+	}
+
+	parm.v_action = action;
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	addattr_l(n, MAX_MSG, TCA_VLAN_PARMS, &parm, sizeof(parm));
+	if (id_set)
+		addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_ID, &id, 2);
+	if (proto_set) {
+		if (proto != htons(ETH_P_8021Q) &&
+		    proto != htons(ETH_P_8021AD)) {
+			fprintf(stderr, "protocol not supported\n");
+			explain();
+			return -1;
+		}
+
+		addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_PROTOCOL, &proto, 2);
+	}
+	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+
+	*argc_p = argc;
+	*argv_p = argv;
+	return 0;
+}
+
+static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
+{
+	SPRINT_BUF(b1);
+	struct rtattr *tb[TCA_VLAN_MAX + 1];
+	__u16 val;
+	struct tc_vlan *parm;
+
+	if (arg == NULL)
+		return -1;
+
+	parse_rtattr_nested(tb, TCA_VLAN_MAX, arg);
+
+	if (!tb[TCA_VLAN_PARMS]) {
+		fprintf(f, "[NULL vlan parameters]");
+		return -1;
+	}
+	parm = RTA_DATA(tb[TCA_VLAN_PARMS]);
+
+	fprintf(f, " vlan");
+
+	switch(parm->v_action) {
+	case TCA_VLAN_ACT_POP:
+		fprintf(f, " pop");
+		break;
+	case TCA_VLAN_ACT_PUSH:
+		fprintf(f, " push");
+		if (tb[TCA_VLAN_PUSH_VLAN_ID]) {
+			val = rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
+			fprintf(f, " id %u", val);
+		}
+		if (tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]) {
+			fprintf(f, " protocol %s",
+				ll_proto_n2a(rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]),
+					     b1, sizeof(b1)));
+		}
+		break;
+	}
+
+	fprintf(f, "\n\t index %d ref %d bind %d", parm->index, parm->refcnt,
+		parm->bindcnt);
+
+	if (show_stats) {
+		if (tb[TCA_VLAN_TM]) {
+			struct tcf_t *tm = RTA_DATA(tb[TCA_VLAN_TM]);
+			print_tm(f, tm);
+		}
+	}
+
+	fprintf(f, "\n ");
+
+	return 0;
+}
+
+struct action_util vlan_action_util = {
+	.id = "vlan",
+	.parse_aopt = parse_vlan,
+	.print_aopt = print_vlan,
+};
-- 
1.9.3

^ permalink raw reply related

* Re: [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates 2014-11-18
From: Jeff Kirsher @ 2014-11-18 21:45 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20141118.133829.2246481313976384106.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

On Tue, 2014-11-18 at 13:38 -0500, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Tue, 18 Nov 2014 01:25:20 -0800
> 
> > This series contains updates to i40e only.
> > 
> > Shannon provides a patch to clean up the driver to only warn once that
> > PTP is not supported when linked at 100Mbps.
> > 
> > Mitch provides a fix for i40e where the VF interrupt processing takes
> > a long time and it is possible that we could lose a VFLR event if it
> > happens while processing a VFLR on another VF.  To correct this situation,
> > we enable the VFLR interrupt cause before we begin processing any pending
> > resets.
> > 
> > Neerav provides several patches to update DCB support in i40e.  When
> > there are DCB configuration changes based on DCBx, the firmware suspends
> > the port's Tx and generates an event to the PF.  The PF is then
> > responsible to reconfigure the PF VSIs and switching topology as per the
> > updated DCB configuration and then resume the port's Tx by calling the
> > "Resume Port Tx" AQ command, so add this call to the flow that handles
> > DCB re-configuration in the PF.  Allow the driver to query and use DCB
> > configuration from firmware when firmware DCBx agent is in CEE mode.
> > Add a check whether LLDP Agent's default AdminStatus is enabled or
> > disabled on a given port, and sets DCBx status to disabled if the
> > status is disabled.  Fix an issue when the port TC configuration
> > changes as a result of DCBx and the driver modifies the enabled TCs for
> > the VEBs it manages but does not update the enabled_tc value that 
> > was cached on a per VEB basis.  Add a new PF state so that if a port's
> > Tx is in suspended state the Tx queue disable flow would just put the
> > request for the queue to be disabled and return without waiting for the
> > queue to be actually disabled.  Allows the driver to enable/disable
> > the XPS based on the number of TCs being enabled for the given VSI.
> 
> Pulled, thanks Jeff.
> 
> > v2: Dropped patch "i40e: Handle a single mss packet with more than 8 frags"
> >     while we rework the patch after we test a bit more based on feedback from 
> >     Eric Dumazet.
> 
> Since, as Eric showed, it's easy to hang the card with a simple
> netperf command line due to this issue, you might want to consider
> sending that fix to 'net' instead.

Got it, I have ear marked the revised patch for 'net'.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH RESEND] fix #51791 - bug? mac 00:00:00:00:00:00 with natsemi DP83815 after driver load
From: Roland Kletzing @ 2014-11-18 21:51 UTC (permalink / raw)
  To: netdev; +Cc: David Miller

This one should fix Bugzilla #51791 

Natsemi driver does not read MAC correctly from eeprom, while natsemi-diag
from nictools-pci does. Apparently, it`s a timing issue in the kernel driver.

According to ftp://ftp.gwdg.de/pub/linux/misc/donald.becker/diag/natsemi-diag.c
, eeprom_delay(ee_addr) is defined as follows:

/* Delay between EEPROM clock transitions.
This flushes the write buffer to prevent quick double-writes.
*/
#define eeprom_delay(ee_addr) inl(ee_addr); inl(ee_addr)

while in the natsemi linux kernel driver, the delay is done this way :

#define eeprom_delay(ee_addr) readl(ee_addr)

, which results in the MAC being all zero`s.

So i simply added a second readl() to increase delay (instead of turning into 
inl() as proposed before). This may look a little bit ugly, but it`s fixing the
problem for me.

I´m not sure how many natsemi users being left on this planet (probably few),
but i guess this change does not do any harm on platforms where the driver does
not behave buggy, so please consider adding it to mainline/stable/longterm.

regards
Roland


Signed-off-by: Roland Kletzing <devzero@web.de>

---
drivers/net/ethernet/natsemi/natsemi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/natsemi/natsemi.c b/drivers/net/ethernet/natsemi/natsemi.c
index b83f7c0..96f0029 100644
--- a/drivers/net/ethernet/natsemi/natsemi.c
+++ b/drivers/net/ethernet/natsemi/natsemi.c
@@ -987,7 +987,7 @@ static int natsemi_probe1(struct pci_dev *pdev, const struct pci_device_id *ent)
    The old method of using an ISA access as a delay, __SLOW_DOWN_IO__, is
    deprecated.
 */
-#define eeprom_delay(ee_addr)  readl(ee_addr)
+#define eeprom_delay(ee_addr)  readl(ee_addr);  readl(ee_addr)

 #define EE_Write0 (EE_ChipSelect)
 #define EE_Write1 (EE_ChipSelect | EE_DataIn)
--

^ permalink raw reply related

* Re: [patch net-next v3 0/9] sched: introduce vlan action
From: Or Gerlitz @ 2014-11-18 21:54 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Pravin Shelar,
	Tom Herbert, Eric Dumazet, willemb, Daniel Borkmann,
	Michael S. Tsirkin <mst@redhat.com> (mst@redhat.com),
	Florian Westphal, Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <1416346664-9290-1-git-send-email-jiri@resnulli.us>

On Tue, Nov 18, 2014 at 11:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:

Hi Jiri (WB from your vacation),

Sorry for the a bit of cold shower, but empty cover letter and no sign for

V2 --> V3
[...]

V1 --> V2
[...]

V0 --> V1
[...]


tracking in the 0/N patch, doesn't allow for proper review, I would say...

Or.

> Jiri Pirko (9):
>   openvswitch: actions: use skb_postpull_rcsum when possible
>   vlan: make __vlan_hwaccel_put_tag return void
>   vlan: kill vlan_put_tag helper
>   vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
>   vlan: introduce *vlan_hwaccel_push_inside helpers
>   vlan: introduce __vlan_insert_tag helper which does not free skb
>   net: move make_writable helper into common code
>   net: move vlan pop/push functions into common code
>   sched: introduce vlan action
>
>  drivers/net/bonding/bond_alb.c              |  17 +--
>  drivers/net/bonding/bond_main.c             |  12 +-
>  drivers/net/ethernet/emulex/benet/be_main.c |   6 +-
>  drivers/net/usb/cdc_mbim.c                  |   2 +-
>  drivers/net/vxlan.c                         |  22 +--
>  drivers/scsi/fcoe/fcoe.c                    |   6 +-
>  include/linux/if_vlan.h                     | 107 +++++++++-----
>  include/linux/skbuff.h                      |   3 +
>  include/net/tc_act/tc_vlan.h                |  27 ++++
>  include/uapi/linux/tc_act/tc_vlan.h         |  35 +++++
>  net/8021q/vlan_dev.c                        |   2 +-
>  net/bridge/br_vlan.c                        |   4 +-
>  net/core/dev.c                              |   8 +-
>  net/core/netpoll.c                          |   4 +-
>  net/core/skbuff.c                           | 107 ++++++++++++++
>  net/ipv4/geneve.c                           |  12 +-
>  net/openvswitch/actions.c                   | 130 ++++-------------
>  net/openvswitch/datapath.c                  |   5 +-
>  net/openvswitch/vport-gre.c                 |  12 +-
>  net/sched/Kconfig                           |  11 ++
>  net/sched/Makefile                          |   1 +
>  net/sched/act_vlan.c                        | 207 ++++++++++++++++++++++++++++
>  22 files changed, 525 insertions(+), 215 deletions(-)
>  create mode 100644 include/net/tc_act/tc_vlan.h
>  create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
>  create mode 100644 net/sched/act_vlan.c
>
> --
> 1.9.3
>
> --
> 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

* Re: [patch net-next v3 0/9] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-18 22:04 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Pravin Shelar,
	Tom Herbert, Eric Dumazet, willemb, Daniel Borkmann,
	Michael S. Tsirkin <mst@redhat.com> (mst@redhat.com),
	Florian Westphal, Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <CAJ3xEMhX0RnykPKJ3VHdn8oez=RHnOWTOFpRaMEYJtr96_AMNA@mail.gmail.com>

Tue, Nov 18, 2014 at 10:54:05PM CET, gerlitz.or@gmail.com wrote:
>On Tue, Nov 18, 2014 at 11:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>
>Hi Jiri (WB from your vacation),
>
>Sorry for the a bit of cold shower, but empty cover letter and no sign for
>
>V2 --> V3
>[...]
>
>V1 --> V2
>[...]
>
>V0 --> V1
>[...]


See individual patches. If there's a change in the patch, these notes
are there.

>
>
>tracking in the 0/N patch, doesn't allow for proper review, I would say...

Cover letter servers only for grouping here. (previous 2 versions did not
have that). See descriptions in individual patches.

>
>Or.
>
>> Jiri Pirko (9):
>>   openvswitch: actions: use skb_postpull_rcsum when possible
>>   vlan: make __vlan_hwaccel_put_tag return void
>>   vlan: kill vlan_put_tag helper
>>   vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
>>   vlan: introduce *vlan_hwaccel_push_inside helpers
>>   vlan: introduce __vlan_insert_tag helper which does not free skb
>>   net: move make_writable helper into common code
>>   net: move vlan pop/push functions into common code
>>   sched: introduce vlan action
>>
>>  drivers/net/bonding/bond_alb.c              |  17 +--
>>  drivers/net/bonding/bond_main.c             |  12 +-
>>  drivers/net/ethernet/emulex/benet/be_main.c |   6 +-
>>  drivers/net/usb/cdc_mbim.c                  |   2 +-
>>  drivers/net/vxlan.c                         |  22 +--
>>  drivers/scsi/fcoe/fcoe.c                    |   6 +-
>>  include/linux/if_vlan.h                     | 107 +++++++++-----
>>  include/linux/skbuff.h                      |   3 +
>>  include/net/tc_act/tc_vlan.h                |  27 ++++
>>  include/uapi/linux/tc_act/tc_vlan.h         |  35 +++++
>>  net/8021q/vlan_dev.c                        |   2 +-
>>  net/bridge/br_vlan.c                        |   4 +-
>>  net/core/dev.c                              |   8 +-
>>  net/core/netpoll.c                          |   4 +-
>>  net/core/skbuff.c                           | 107 ++++++++++++++
>>  net/ipv4/geneve.c                           |  12 +-
>>  net/openvswitch/actions.c                   | 130 ++++-------------
>>  net/openvswitch/datapath.c                  |   5 +-
>>  net/openvswitch/vport-gre.c                 |  12 +-
>>  net/sched/Kconfig                           |  11 ++
>>  net/sched/Makefile                          |   1 +
>>  net/sched/act_vlan.c                        | 207 ++++++++++++++++++++++++++++
>>  22 files changed, 525 insertions(+), 215 deletions(-)
>>  create mode 100644 include/net/tc_act/tc_vlan.h
>>  create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
>>  create mode 100644 net/sched/act_vlan.c
>>
>> --
>> 1.9.3
>>
>> --
>> 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

* Re: ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Jiri Bohac @ 2014-11-18 22:10 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Jiri Bohac, Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <8698539.evYG7fs8jS@wuerfel>

On Tue, Nov 18, 2014 at 02:37:26PM +0100, Arnd Bergmann wrote:
> Does ipxrtr_route_packet() actually sleep while waiting for the network,
> or is it possible that you only need to change the recvmsg path?

You're right. 
In fact, it can sleep in sock_alloc_send_skb(), but my patch does
not fix this - it releases the lock after that.
So let's ignore that for now, I'll send a V2 modifying only
ipx_recvmsg().

> >  	if (sock_flag(sk, SOCK_ZAPPED))
> > -		goto out;
> > +		goto out_release;
> >  
> > +	release_sock(sk);
> >  	skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
> >  				flags & MSG_DONTWAIT, &rc);
> >  	if (!skb) {
> 
> Same thing here: I think your patch could be simplified if you just
> release the socket lock before calling skb_recv_datagram and get
> it back afterwards, 

It would simplify the code a little to just get the lock again.
But do we really want to optimize for source code size at the cost of
taking locks that are not necessarry?

> and it would be much simpler if you could show that the lock is
> not needed at all.

At least the ipxitf_insert_socket() inside __ipx_bind() looks
like it must be protected not to corrupt the intrfc->if_sklist.
I am not familiar with the code, so there may be other things.

Thanks for the review!

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

^ permalink raw reply

* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Cong Wang @ 2014-11-18 22:12 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev
In-Reply-To: <1416307059-32732-1-git-send-email-lucien.xin@gmail.com>

On Tue, Nov 18, 2014 at 2:37 AM, Xin Long <lucien.xin@gmail.com> wrote:
> Now the vti_link_ops do not point the .dellink, rtnl_newlink will invoke
> the default function, unregister_netdevice_queue, which will cause the
> dev to unregister later. so when we delete a vti device, the net_device
> will be removed, but the tunnel still in the tunnel list. then, we add
> a new vti, in ip_tunnel_find():
>
>         hlist_for_each_entry_rcu(t, head, hash_node) {
>                 if (local == t->parms.iph.saddr &&
>                     remote == t->parms.iph.daddr &&
>                     link == t->parms.link &&
>                     type == t->dev->type &&
>                     ip_tunnel_key_match(&t->parms, flags, key))
>                         break;
>         }
>
> the dev of ip_tunnel *t may be null because of unregister_netdevice_queue
> motioned above. so the panic will happen:


Are you sure? We call ->ndo_uninit() in unregister path, and vti tunnel
has ip_tunnel_uninit() which removes non-fb tunnel from that list.

>
> [ 3835.072977] IP: [<ffffffffa04103fd>] ip_tunnel_find+0x9d/0xc0 [ip_tunnel]
> [ 3835.073008] PGD b2c21067 PUD b7277067 PMD 0
> [ 3835.073008] Oops: 0000 [#1] SMP
> .....
> [ 3835.073008] Stack:
> [ 3835.073008]  ffff8800b72d77f0 ffffffffa0411924 ffff8800bb956000 ffff8800b72d78e0
> [ 3835.073008]  ffff8800b72d78a0 0000000000000000 ffffffffa040d100 ffff8800b72d7858
> [ 3835.073008]  ffffffffa040b2e3 0000000000000000 0000000000000000 0000000000000000
> [ 3835.073008] Call Trace:
> [ 3835.073008]  [<ffffffffa0411924>] ip_tunnel_newlink+0x64/0x160 [ip_tunnel]
> [ 3835.073008]  [<ffffffffa040b2e3>] vti_newlink+0x43/0x70 [ip_vti]
> [ 3835.073008]  [<ffffffff8150d4da>] rtnl_newlink+0x4fa/0x5f0
> [ 3835.073008]  [<ffffffff812f68bb>] ? nla_strlcpy+0x5b/0x70
> [ 3835.073008]  [<ffffffff81508fb0>] ? rtnl_link_ops_get+0x40/0x60
> [ 3835.073008]  [<ffffffff8150d11f>] ? rtnl_newlink+0x13f/0x5f0
> [ 3835.073008]  [<ffffffff81509cf4>] rtnetlink_rcv_msg+0xa4/0x270
> [ 3835.073008]  [<ffffffff8126adf5>] ? sock_has_perm+0x75/0x90
> [ 3835.073008]  [<ffffffff81509c50>] ? rtnetlink_rcv+0x30/0x30
> [ 3835.073008]  [<ffffffff81529e39>] netlink_rcv_skb+0xa9/0xc0
> [ 3835.073008]  [<ffffffff81509c48>] rtnetlink_rcv+0x28/0x30
> ....
>
> the reproduction can be like this:
>
> modprobe ip_vti
> ip link del ip_vti0 type vti
> ip link add ip_vti0 type vti
> rmmod ip_vti
>
> do that one or more time, kernel will panic.


>From your reproducer, I suspect the problem is we might miss
some clean up for the fb device ip_vti0, not for every vti tunnel.
Or maybe some race condition. Still digging.

^ permalink raw reply

* [PATCH v2] ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Jiri Bohac @ 2014-11-18 22:24 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: jbohac, Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <20141118221057.GA13473@midget.suse.cz>

This fixes an old regression introduced by commit
b0d0d915 (ipx: remove the BKL).

When a recvmsg syscall blocks waiting for new data, no data can be sent on the
same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.

This breaks mars-nwe (NetWare emulator):
- the ncpserv process reads the request using recvmsg
- ncpserv forks and spawns nwconn
- ncpserv calls a (blocking) recvmsg and waits for new requests
- nwconn deadlocks in sendmsg on the same socket 

Commit b0d0d915 has simply replaced BKL locking with
lock_sock/release_sock. Unlike now, BKL got unlocked while
sleeping, so a blocking recvmsg did not block a concurrent
sendmsg.

Only keep the socket locked while actually working with the socket data and
release it prior to calling skb_recv_datagram(). 


Signed-off-by: Jiri Bohac <jbohac@suse.cz>

diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 91729b8..1e0d796 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1776,20 +1780,21 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
 #ifdef CONFIG_IPX_INTERN
 		rc = -ENETDOWN;
 		if (!ipxs->intrfc)
-			goto out; /* Someone zonked the iface */
+			goto out_release; /* Someone zonked the iface */
 		memcpy(uaddr.sipx_node, ipxs->intrfc->if_node, IPX_NODE_LEN);
 #endif	/* CONFIG_IPX_INTERN */
 
 		rc = __ipx_bind(sock, (struct sockaddr *)&uaddr,
 			      sizeof(struct sockaddr_ipx));
 		if (rc)
-			goto out;
+			goto out_release;
 	}
 
 	rc = -ENOTCONN;
 	if (sock_flag(sk, SOCK_ZAPPED))
-		goto out;
+		goto out_release;
 
+	release_sock(sk);
 	skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
 				flags & MSG_DONTWAIT, &rc);
 	if (!skb) {
@@ -1807,8 +1812,10 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
 
 	rc = skb_copy_datagram_iovec(skb, sizeof(struct ipxhdr), msg->msg_iov,
 				     copied);
-	if (rc)
-		goto out_free;
+	if (rc) {
+		skb_free_datagram(sk, skb);
+		goto out;
+	}
 	if (skb->tstamp.tv64)
 		sk->sk_stamp = skb->tstamp;
 
@@ -1822,11 +1829,11 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
 		msg->msg_namelen	= sizeof(*sipx);
 	}
 	rc = copied;
+	goto out;
 
-out_free:
-	skb_free_datagram(sk, skb);
-out:
+out_release:
 	release_sock(sk);
+out:
 	return rc;
 }
 
-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

^ permalink raw reply related

* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-18 22:47 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch@vger.kernel.org, Network Development,
	Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
	Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
	Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
	matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
	Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
	Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <CA+55aFw3ygP8UEF1vidWdjBUEkqXOELCPkpjLMD-X=vXXQEGOQ@mail.gmail.com>

On 11/18/2014 12:53 PM, Linus Torvalds wrote:
> On Tue, Nov 18, 2014 at 9:28 AM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>> These patches introduce two new primitives for synchronizing cache coherent
>> memory writes and reads.  These two new primitives are:
>>
>>          coherent_rmb()
>>          coherent_wmb()
>
> So I'm still not convinced about the name. I don't hate it, but if you
> ever want to do "read_acquire", then that whole "coherent_" thing does
> make for a big mouthful. I don't see why "dma" isn't simpler and more
> to the point, and has the advantage of lining up (in documentation
> etc) with "smp".

The problem is DMA is a broad brush.  There are multiple cases I can 
think of where DMA does not represent coherent memory.

> Why would you ever use "coherent_xyz()" on something that isn't about
> dma? If it's cache-coherent memory without DMA, you'd use "smp_xyz()",
> so I really do prefer that whole "dma-vs-smp" issue, because it talks
> about what is actually the important issue. All sane memory is
> coherent, after all (and if it isn't, you have other issues than
> memory ordering).
>
>                    Linus

This barrier only applies to a subset of DMA memory types.  So yes, 
"coherent_xyz()" only applies to DMA, but not all DMA memory is coherent 
as it could be a non-coherent or streaming DMA mapping.

One spot where the name makes sense is in the headers themselves.  To 
avoid duplication of definitions in several spots if CONFIG_SMP was 
defined I simply defined smp_xyz() as coherent_xyz().  Defining it as 
dma_xyz() might have made that more difficult to read in terms of what 
was going on.

- Alex

^ permalink raw reply

* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Linus Torvalds @ 2014-11-18 23:06 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: linux-arch@vger.kernel.org, Network Development,
	Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
	Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
	Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
	matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
	Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
	Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <546BCC73.3050903@redhat.com>

On Tue, Nov 18, 2014 at 2:47 PM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
>
> The problem is DMA is a broad brush.  There are multiple cases I can think
> of where DMA does not represent coherent memory.

.. and I already addressed that, in the thing you even included:

>> about what is actually the important issue. All sane memory is
>> coherent, after all (and if it isn't, you have other issues than
>> memory ordering).

The thing is, if the DMA isn't coherent, nobody is going to care about
the memory barriers anyway. You have bigger issues.

And your argument is that "dma" is bigger than this issue. *MY*
argument is that "coherent" is bigger than this issue. There's tons of
coherent memory that is not about DMA, the same way that there is DMA
memory that isn't coherent.

See? The two are 100% equivalent. Except "dma" is just three letters,
and matches "smp" both visually and in use (SMP memory is "coherent"
too - yes, you can - and crap architectures do - have incoherent
caches due to virtual aliases etc, but exactly as with DMA, if you
have incoherent SMP, you have bigger issues than the barriers).

And yes, you could call it "coherent_dma_read_memory_barrier()", and
it would be very descriptive. It would also drive everybody crazy.

So I argue for "dma_mb()" pairing with "smp_mb()" from a naming
standpoint. It just *describes* the problem better. Look at the
drivers, it's very much about the devices doing DMA to memory, and our
ordering.

To be even more clear: nobody sane cares about the "coherent" part,
because only insane horrible crap architectures have incoherent memory
in the first place, and sane people run away screaming from that
steaming pile of sh*t.

Just look at some of the drivers you actually *use* this in. They are
for intel hardware, they presumably would never even work in the first
place without cache-coherent DMA. Why do you think that "coherent" is
so important?

                       Linus

^ permalink raw reply

* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Cong Wang @ 2014-11-18 23:19 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev
In-Reply-To: <CAHA+R7NKdS_j2MC+-Bcw=hFERODiMw1Z6ju6yw5TvXXHbTXK5A@mail.gmail.com>

On Tue, Nov 18, 2014 at 2:12 PM, Cong Wang <cwang@twopensource.com> wrote:
> On Tue, Nov 18, 2014 at 2:37 AM, Xin Long <lucien.xin@gmail.com> wrote:
>> Now the vti_link_ops do not point the .dellink, rtnl_newlink will invoke
>> the default function, unregister_netdevice_queue, which will cause the
>> dev to unregister later. so when we delete a vti device, the net_device
>> will be removed, but the tunnel still in the tunnel list. then, we add
>> a new vti, in ip_tunnel_find():
>>
>>         hlist_for_each_entry_rcu(t, head, hash_node) {
>>                 if (local == t->parms.iph.saddr &&
>>                     remote == t->parms.iph.daddr &&
>>                     link == t->parms.link &&
>>                     type == t->dev->type &&
>>                     ip_tunnel_key_match(&t->parms, flags, key))
>>                         break;
>>         }
>>
>> the dev of ip_tunnel *t may be null because of unregister_netdevice_queue
>> motioned above. so the panic will happen:
>
>
> Are you sure? We call ->ndo_uninit() in unregister path, and vti tunnel
> has ip_tunnel_uninit() which removes non-fb tunnel from that list.
>

Hmm, your fix is correct, but the description is wrong, we should just skip
the unregister of fb tunnel device like other tunnels. Without ->dellink()
the fb device is still removed from the global netdev list, this needs to skip,
otherwise ip_vti0 will disappear from your system.

^ permalink raw reply

* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Cong Wang @ 2014-11-18 23:25 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: Xin Long, network dev, Steffen Klassert
In-Reply-To: <546B8093.4080507@6wind.com>

On Tue, Nov 18, 2014 at 9:23 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> A quick look at the ipv6 side seems to show that there is the same problem.
> Can
> you provide the IPv6 patch too?
>

IPv6 doesn't use ip_tunnel library, so needs to provide its own implementation:

diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index ec84d03..3ae094b 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -911,6 +911,16 @@ static int vti6_newlink(struct net *src_net,
struct net_device *dev,
        return vti6_tnl_create2(dev);
 }

+static void vti6_dellink(struct net_device *dev, struct list_head *head)
+{
+       struct net *net = dev_net(dev);
+       struct vti6_net *ip6n = net_generic(net, vti6_net_id);
+
+       if (dev != ip6n->fb_tnl_dev)
+               unregister_netdevice_queue(dev, head);
+}
+
+
 static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
                           struct nlattr *data[])
 {
@@ -986,6 +996,7 @@ static struct rtnl_link_ops vti6_link_ops __read_mostly = {
        .setup          = vti6_dev_setup,
        .validate       = vti6_validate,
        .newlink        = vti6_newlink,
+       .dellink        = vti6_dellink,
        .changelink     = vti6_changelink,
        .get_size       = vti6_get_size,
        .fill_info      = vti6_fill_info,

^ permalink raw reply related

* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-19  0:07 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch@vger.kernel.org, Network Development,
	Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
	Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
	Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
	matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
	Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
	Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <CA+55aFwHUQv+zBxyN41Y4MDyRXbQ84H8zc01O6O_eg9ZvoqYiQ@mail.gmail.com>



On 11/18/2014 03:06 PM, Linus Torvalds wrote:
> On Tue, Nov 18, 2014 at 2:47 PM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>>
>> The problem is DMA is a broad brush.  There are multiple cases I can think
>> of where DMA does not represent coherent memory.
>
> .. and I already addressed that, in the thing you even included:
>
>>> about what is actually the important issue. All sane memory is
>>> coherent, after all (and if it isn't, you have other issues than
>>> memory ordering).
>
> The thing is, if the DMA isn't coherent, nobody is going to care about
> the memory barriers anyway. You have bigger issues.
>
> And your argument is that "dma" is bigger than this issue. *MY*
> argument is that "coherent" is bigger than this issue. There's tons of
> coherent memory that is not about DMA, the same way that there is DMA
> memory that isn't coherent.
>
> See? The two are 100% equivalent. Except "dma" is just three letters,
> and matches "smp" both visually and in use (SMP memory is "coherent"
> too - yes, you can - and crap architectures do - have incoherent
> caches due to virtual aliases etc, but exactly as with DMA, if you
> have incoherent SMP, you have bigger issues than the barriers).

Actually if anything maybe the crap architectures are a good reason for 
changing the name.  If they can't even do coherent SMP memory then the 
coherent_*mb() could be misleading since they would just be full 
barriers anyway.

> And yes, you could call it "coherent_dma_read_memory_barrier()", and
> it would be very descriptive. It would also drive everybody crazy.

No, I think "dma_wmb__before_coherent_write" would have been much more 
descriptive.  You have to squeeze in that extra underscore somewhere. ;-)

> So I argue for "dma_mb()" pairing with "smp_mb()" from a naming
> standpoint. It just *describes* the problem better. Look at the
> drivers, it's very much about the devices doing DMA to memory, and our
> ordering.
>
> To be even more clear: nobody sane cares about the "coherent" part,
> because only insane horrible crap architectures have incoherent memory
> in the first place, and sane people run away screaming from that
> steaming pile of sh*t.

I think that is part of my reluctance.  I didn't even want it implied 
that the barriers could be used with that kind of stuff.

> Just look at some of the drivers you actually *use* this in. They are
> for intel hardware, they presumably would never even work in the first
> place without cache-coherent DMA. Why do you think that "coherent" is
> so important?
>
>                         Linus

v5 should be up shortly after a quick pass with sed to do the 
find/replace, clean up any whitespace issues, and a quick run through 
some cross compiling scripts just to make sure I didn't screw anything up.

- Alex

^ permalink raw reply

* Re: [PATCH] ipv6: delete protocol when cleanup
From: Duan Jiong @ 2014-11-19  1:10 UTC (permalink / raw)
  To: Cong Wang; +Cc: David Miller, netdev
In-Reply-To: <CAHA+R7PNmac7KQpSK-vO9zVd3+926f6n_GpaGrbpBnSn98xiyQ@mail.gmail.com>

On 11/19/2014 02:56 AM, Cong Wang wrote:
> On Tue, Nov 18, 2014 at 2:35 AM, Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:
>>
>> pim6_protocol was added when initiation, but it not deleted.
>>
>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>> ---
>>  net/ipv6/ip6mr.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
>> index 0171f08..6dcdecd 100644
>> --- a/net/ipv6/ip6mr.c
>> +++ b/net/ipv6/ip6mr.c
>> @@ -1439,6 +1439,9 @@ reg_pernet_fail:
>>
>>  void ip6_mr_cleanup(void)
>>  {
>> +#ifdef CONFIG_IPV6_PIMSM_V2
>> +       inet6_del_protocol(&pim6_protocol, IPPROTO_PIM);
>> +#endif
>>         unregister_netdevice_notifier(&ip6_mr_notifier);
>>         unregister_pernet_subsys(&ip6mr_net_ops);
>>         kmem_cache_destroy(mrt_cachep);
> 
> 
> Looks like rtnl_unregister() is missing as well.
> 

Thanks for pointing this, i will send v2.

Thanks,
  Duan

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox