Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/2] net: Add Transparent Ethernet Bridging GRO support.
From: Jesse Gross @ 2014-12-31  3:10 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Currently the only tunnel protocol that supports GRO with encapsulated
Ethernet is VXLAN. This pulls out the Ethernet code into a proper layer
so that it can be used by other tunnel protocols such as GRE and Geneve.

Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 drivers/net/vxlan.c         | 53 +++-----------------------
 include/linux/etherdevice.h |  4 ++
 net/ethernet/eth.c          | 92 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 47 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 7fbd89f..2ab0922 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -549,10 +549,7 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
 {
 	struct sk_buff *p, **pp = NULL;
 	struct vxlanhdr *vh, *vh2;
-	struct ethhdr *eh, *eh2;
-	unsigned int hlen, off_vx, off_eth;
-	const struct packet_offload *ptype;
-	__be16 type;
+	unsigned int hlen, off_vx;
 	int flush = 1;
 
 	off_vx = skb_gro_offset(skb);
@@ -563,17 +560,6 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
 		if (unlikely(!vh))
 			goto out;
 	}
-	skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
-	skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
-
-	off_eth = skb_gro_offset(skb);
-	hlen = off_eth + sizeof(*eh);
-	eh   = skb_gro_header_fast(skb, off_eth);
-	if (skb_gro_header_hard(skb, hlen)) {
-		eh = skb_gro_header_slow(skb, hlen, off_eth);
-		if (unlikely(!eh))
-			goto out;
-	}
 
 	flush = 0;
 
@@ -582,28 +568,16 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
 			continue;
 
 		vh2 = (struct vxlanhdr *)(p->data + off_vx);
-		eh2 = (struct ethhdr   *)(p->data + off_eth);
-		if (vh->vx_vni != vh2->vx_vni || compare_ether_header(eh, eh2)) {
+		if (vh->vx_vni != vh2->vx_vni) {
 			NAPI_GRO_CB(p)->same_flow = 0;
 			continue;
 		}
 	}
 
-	type = eh->h_proto;
-
-	rcu_read_lock();
-	ptype = gro_find_receive_by_type(type);
-	if (ptype == NULL) {
-		flush = 1;
-		goto out_unlock;
-	}
-
-	skb_gro_pull(skb, sizeof(*eh)); /* pull inner eth header */
-	skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
-	pp = ptype->callbacks.gro_receive(head, skb);
+	skb_gro_pull(skb, sizeof(struct vxlanhdr));
+	skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
+	pp = eth_gro_receive(head, skb);
 
-out_unlock:
-	rcu_read_unlock();
 out:
 	NAPI_GRO_CB(skb)->flush |= flush;
 
@@ -612,24 +586,9 @@ out:
 
 static int vxlan_gro_complete(struct sk_buff *skb, int nhoff)
 {
-	struct ethhdr *eh;
-	struct packet_offload *ptype;
-	__be16 type;
-	int vxlan_len  = sizeof(struct vxlanhdr) + sizeof(struct ethhdr);
-	int err = -ENOSYS;
-
 	udp_tunnel_gro_complete(skb, nhoff);
 
-	eh = (struct ethhdr *)(skb->data + nhoff + sizeof(struct vxlanhdr));
-	type = eh->h_proto;
-
-	rcu_read_lock();
-	ptype = gro_find_complete_by_type(type);
-	if (ptype != NULL)
-		err = ptype->callbacks.gro_complete(skb, nhoff + vxlan_len);
-
-	rcu_read_unlock();
-	return err;
+	return eth_gro_complete(skb, nhoff + sizeof(struct vxlanhdr));
 }
 
 /* Notify netdevs that UDP port started listening */
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 41c891d..1d869d1 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -52,6 +52,10 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
 #define alloc_etherdev(sizeof_priv) alloc_etherdev_mq(sizeof_priv, 1)
 #define alloc_etherdev_mq(sizeof_priv, count) alloc_etherdev_mqs(sizeof_priv, count, count)
 
+struct sk_buff **eth_gro_receive(struct sk_buff **head,
+				 struct sk_buff *skb);
+int eth_gro_complete(struct sk_buff *skb, int nhoff);
+
 /* Reserved Ethernet Addresses per IEEE 802.1Q */
 static const u8 eth_reserved_addr_base[ETH_ALEN] __aligned(2) =
 { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 33a140e..238f38d 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -424,3 +424,95 @@ ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len)
 	return scnprintf(buf, PAGE_SIZE, "%*phC\n", len, addr);
 }
 EXPORT_SYMBOL(sysfs_format_mac);
+
+struct sk_buff **eth_gro_receive(struct sk_buff **head,
+				 struct sk_buff *skb)
+{
+	struct sk_buff *p, **pp = NULL;
+	struct ethhdr *eh, *eh2;
+	unsigned int hlen, off_eth;
+	const struct packet_offload *ptype;
+	__be16 type;
+	int flush = 1;
+
+	off_eth = skb_gro_offset(skb);
+	hlen = off_eth + sizeof(*eh);
+	eh = skb_gro_header_fast(skb, off_eth);
+	if (skb_gro_header_hard(skb, hlen)) {
+		eh = skb_gro_header_slow(skb, hlen, off_eth);
+		if (unlikely(!eh))
+			goto out;
+	}
+
+	flush = 0;
+
+	for (p = *head; p; p = p->next) {
+		if (!NAPI_GRO_CB(p)->same_flow)
+			continue;
+
+		eh2 = (struct ethhdr *)(p->data + off_eth);
+		if (compare_ether_header(eh, eh2)) {
+			NAPI_GRO_CB(p)->same_flow = 0;
+			continue;
+		}
+	}
+
+	type = eh->h_proto;
+
+	rcu_read_lock();
+	ptype = gro_find_receive_by_type(type);
+	if (ptype == NULL) {
+		flush = 1;
+		goto out_unlock;
+	}
+
+	skb_gro_pull(skb, sizeof(*eh));
+	skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
+	pp = ptype->callbacks.gro_receive(head, skb);
+
+out_unlock:
+	rcu_read_unlock();
+out:
+	NAPI_GRO_CB(skb)->flush |= flush;
+
+	return pp;
+}
+EXPORT_SYMBOL(eth_gro_receive);
+
+int eth_gro_complete(struct sk_buff *skb, int nhoff)
+{
+	struct ethhdr *eh = (struct ethhdr *)(skb->data + nhoff);
+	__be16 type = eh->h_proto;
+	struct packet_offload *ptype;
+	int err = -ENOSYS;
+
+	if (skb->encapsulation)
+		skb_set_inner_mac_header(skb, nhoff);
+
+	rcu_read_lock();
+	ptype = gro_find_complete_by_type(type);
+	if (ptype != NULL)
+		err = ptype->callbacks.gro_complete(skb, nhoff +
+						    sizeof(struct ethhdr));
+
+	rcu_read_unlock();
+	return err;
+}
+EXPORT_SYMBOL(eth_gro_complete);
+
+static struct packet_offload eth_packet_offload __read_mostly = {
+	.type = cpu_to_be16(ETH_P_TEB),
+	.callbacks = {
+		.gro_receive = eth_gro_receive,
+		.gro_complete = eth_gro_complete,
+	},
+};
+
+static int __init eth_offload_init(void)
+{
+	dev_add_offload(&eth_packet_offload);
+
+	return 0;
+}
+
+fs_initcall(eth_offload_init);
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 2/2] geneve: Add Geneve GRO support
From: Jesse Gross @ 2014-12-31  3:10 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Joe Stringer
In-Reply-To: <1419995416-29987-1-git-send-email-jesse@nicira.com>

From: Joe Stringer <joestringer@nicira.com>

This results in an approximately 30% increase in throughput
when handling encapsulated bulk traffic.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 net/ipv4/geneve.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 95 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index 394a200..19e256e 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -149,6 +149,99 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 }
 EXPORT_SYMBOL_GPL(geneve_xmit_skb);
 
+static int geneve_hlen(struct genevehdr *gh)
+{
+	return sizeof(*gh) + gh->opt_len * 4;
+}
+
+static struct sk_buff **geneve_gro_receive(struct sk_buff **head,
+					   struct sk_buff *skb)
+{
+	struct sk_buff *p, **pp = NULL;
+	struct genevehdr *gh, *gh2;
+	unsigned int hlen, gh_len, off_gnv;
+	const struct packet_offload *ptype;
+	__be16 type;
+	int flush = 1;
+
+	off_gnv = skb_gro_offset(skb);
+	hlen = off_gnv + sizeof(*gh);
+	gh = skb_gro_header_fast(skb, off_gnv);
+	if (skb_gro_header_hard(skb, hlen)) {
+		gh = skb_gro_header_slow(skb, hlen, off_gnv);
+		if (unlikely(!gh))
+			goto out;
+	}
+
+	if (gh->ver != GENEVE_VER || gh->oam)
+		goto out;
+	gh_len = geneve_hlen(gh);
+
+	hlen = off_gnv + gh_len;
+	if (skb_gro_header_hard(skb, hlen)) {
+		gh = skb_gro_header_slow(skb, hlen, off_gnv);
+		if (unlikely(!gh))
+			goto out;
+	}
+
+	flush = 0;
+
+	for (p = *head; p; p = p->next) {
+		if (!NAPI_GRO_CB(p)->same_flow)
+			continue;
+
+		gh2 = (struct genevehdr *)(p->data + off_gnv);
+		if (gh->opt_len != gh2->opt_len ||
+		    memcmp(gh, gh2, gh_len)) {
+			NAPI_GRO_CB(p)->same_flow = 0;
+			continue;
+		}
+	}
+
+	type = gh->proto_type;
+
+	rcu_read_lock();
+	ptype = gro_find_receive_by_type(type);
+	if (ptype == NULL) {
+		flush = 1;
+		goto out_unlock;
+	}
+
+	skb_gro_pull(skb, gh_len);
+	skb_gro_postpull_rcsum(skb, gh, gh_len);
+	pp = ptype->callbacks.gro_receive(head, skb);
+
+out_unlock:
+	rcu_read_unlock();
+out:
+	NAPI_GRO_CB(skb)->flush |= flush;
+
+	return pp;
+}
+
+static int geneve_gro_complete(struct sk_buff *skb, int nhoff)
+{
+	struct genevehdr *gh;
+	struct packet_offload *ptype;
+	__be16 type;
+	int gh_len;
+	int err = -ENOSYS;
+
+	udp_tunnel_gro_complete(skb, nhoff);
+
+	gh = (struct genevehdr *)(skb->data + nhoff);
+	gh_len = geneve_hlen(gh);
+	type = gh->proto_type;
+
+	rcu_read_lock();
+	ptype = gro_find_complete_by_type(type);
+	if (ptype != NULL)
+		err = ptype->callbacks.gro_complete(skb, nhoff + gh_len);
+
+	rcu_read_unlock();
+	return err;
+}
+
 static void geneve_notify_add_rx_port(struct geneve_sock *gs)
 {
 	struct sock *sk = gs->sock->sk;
@@ -278,8 +371,8 @@ static struct geneve_sock *geneve_socket_create(struct net *net, __be16 port,
 
 	/* Initialize the geneve udp offloads structure */
 	gs->udp_offloads.port = port;
-	gs->udp_offloads.callbacks.gro_receive = NULL;
-	gs->udp_offloads.callbacks.gro_complete = NULL;
+	gs->udp_offloads.callbacks.gro_receive  = geneve_gro_receive;
+	gs->udp_offloads.callbacks.gro_complete = geneve_gro_complete;
 
 	spin_lock(&gn->sock_lock);
 	hlist_add_head_rcu(&gs->hlist, gs_head(net, port));
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 23/23 V2 for 3.19]  rtlwifi: Fix error when accessing unmapped memory in skb
From: Larry Finger @ 2014-12-31  3:31 UTC (permalink / raw)
  To: Eric Biggers; +Cc: kvalo, linux-wireless, netdev, Stable
In-Reply-To: <20141231004947.GA2007@zzz>

On 12/30/2014 06:49 PM, Eric Biggers wrote:
> On Sat, Dec 27, 2014 at 02:17:37PM -0600, Larry Finger wrote:
>> These drivers use 9100-byte receive buffers, thus allocating an skb requires
>> an O(3) memory allocation. Under heavy memory loads and fragmentation, such
>> a request can fail. Previous versions of the driver have dropped the packet
>> and reused the old buffer; however, the new version introduced a bug in that
>> it released the old buffer before trying to allocate a new one. The previous
>> method is implemented here.
>
> It looks like in the out-of-memory path, pci_map_single() gets called while the
> skb is still mapped.  Won't this leak the IOMMU mapping?

Good catch. I do not know much about leaking the IOMMU mapping; however it is 
easy to do the unmapping before trying to allocate a new skb.

Thanks,

Larry

^ permalink raw reply

* [PATCH V3 for 3.19]  rtlwifi: Fix error when accessing unmapped memory in skb
From: Larry Finger @ 2014-12-31  3:33 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Larry Finger, netdev, Stable, Eric Biggers

These drivers use 9100-byte receive buffers, thus allocating an skb requires
an O(3) memory allocation. Under heavy memory loads and fragmentation, such
a request can fail. Previous versions of the driver have dropped the packet
and reused the old buffer; however, the new version introduced a bug in that
it released the old buffer before trying to allocate a new one. The previous
method is implemented here. The skb is unmapped before any attempt is made to
allocate another.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>  [v3.18]
Reported-by: Eric Biggers <ebiggers3@gmail.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
---

V2 - Fixes an error in the logic of V1. Realtek is working on a change to
     the RX buffer allocation, but that is likely to be too invasive for
     a fix to -rc or stable. In the meantime, this will help.
v3 - Unmap skb before trying to allocate a new one so as to not leak mapping.

Larry
---

 drivers/net/wireless/rtlwifi/pci.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

Index: wireless-drivers/drivers/net/wireless/rtlwifi/pci.c
===================================================================
--- wireless-drivers.orig/drivers/net/wireless/rtlwifi/pci.c
+++ wireless-drivers/drivers/net/wireless/rtlwifi/pci.c
@@ -666,7 +666,8 @@ tx_status_ok:
 }
 
 static int _rtl_pci_init_one_rxdesc(struct ieee80211_hw *hw,
-				    u8 *entry, int rxring_idx, int desc_idx)
+				    struct sk_buff *new_skb, u8 *entry,
+				    int rxring_idx, int desc_idx)
 {
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
@@ -674,11 +675,15 @@ static int _rtl_pci_init_one_rxdesc(stru
 	u8 tmp_one = 1;
 	struct sk_buff *skb;
 
+	if (likely(new_skb)) {
+		skb = new_skb;
+		goto remap;
+	}
 	skb = dev_alloc_skb(rtlpci->rxbuffersize);
 	if (!skb)
 		return 0;
-	rtlpci->rx_ring[rxring_idx].rx_buf[desc_idx] = skb;
 
+remap:
 	/* just set skb->cb to mapping addr for pci_unmap_single use */
 	*((dma_addr_t *)skb->cb) =
 		pci_map_single(rtlpci->pdev, skb_tail_pointer(skb),
@@ -686,6 +691,7 @@ static int _rtl_pci_init_one_rxdesc(stru
 	bufferaddress = *((dma_addr_t *)skb->cb);
 	if (pci_dma_mapping_error(rtlpci->pdev, bufferaddress))
 		return 0;
+	rtlpci->rx_ring[rxring_idx].rx_buf[desc_idx] = skb;
 	if (rtlpriv->use_new_trx_flow) {
 		rtlpriv->cfg->ops->set_desc(hw, (u8 *)entry, false,
 					    HW_DESC_RX_PREPARE,
@@ -781,6 +787,7 @@ static void _rtl_pci_rx_interrupt(struct
 		/*rx pkt */
 		struct sk_buff *skb = rtlpci->rx_ring[rxring_idx].rx_buf[
 				      rtlpci->rx_ring[rxring_idx].idx];
+		struct sk_buff *new_skb;
 
 		if (rtlpriv->use_new_trx_flow) {
 			rx_remained_cnt =
@@ -807,6 +814,13 @@ static void _rtl_pci_rx_interrupt(struct
 		pci_unmap_single(rtlpci->pdev, *((dma_addr_t *)skb->cb),
 				 rtlpci->rxbuffersize, PCI_DMA_FROMDEVICE);
 
+		/* get a new skb - if fail, old one will be reused */
+		new_skb = dev_alloc_skb(rtlpci->rxbuffersize);
+		if (unlikely(!new_skb)) {
+			pr_err("Allocation of new skb failed in %s\n",
+			       __func__);
+			goto no_new;
+		}
 		if (rtlpriv->use_new_trx_flow) {
 			buffer_desc =
 			  &rtlpci->rx_ring[rxring_idx].buffer_desc
@@ -911,14 +925,16 @@ static void _rtl_pci_rx_interrupt(struct
 			schedule_work(&rtlpriv->works.lps_change_work);
 		}
 end:
+		skb = new_skb;
+no_new:
 		if (rtlpriv->use_new_trx_flow) {
-			_rtl_pci_init_one_rxdesc(hw, (u8 *)buffer_desc,
+			_rtl_pci_init_one_rxdesc(hw, skb, (u8 *)buffer_desc,
 						 rxring_idx,
-					       rtlpci->rx_ring[rxring_idx].idx);
+						 rtlpci->rx_ring[rxring_idx].idx);
 		} else {
-			_rtl_pci_init_one_rxdesc(hw, (u8 *)pdesc, rxring_idx,
+			_rtl_pci_init_one_rxdesc(hw, skb, (u8 *)pdesc,
+						 rxring_idx,
 						 rtlpci->rx_ring[rxring_idx].idx);
-
 			if (rtlpci->rx_ring[rxring_idx].idx ==
 			    rtlpci->rxringcount - 1)
 				rtlpriv->cfg->ops->set_desc(hw, (u8 *)pdesc,
@@ -1307,7 +1323,7 @@ static int _rtl_pci_init_rx_ring(struct
 		rtlpci->rx_ring[rxring_idx].idx = 0;
 		for (i = 0; i < rtlpci->rxringcount; i++) {
 			entry = &rtlpci->rx_ring[rxring_idx].buffer_desc[i];
-			if (!_rtl_pci_init_one_rxdesc(hw, (u8 *)entry,
+			if (!_rtl_pci_init_one_rxdesc(hw, NULL, (u8 *)entry,
 						      rxring_idx, i))
 				return -ENOMEM;
 		}
@@ -1332,7 +1348,7 @@ static int _rtl_pci_init_rx_ring(struct
 
 		for (i = 0; i < rtlpci->rxringcount; i++) {
 			entry = &rtlpci->rx_ring[rxring_idx].desc[i];
-			if (!_rtl_pci_init_one_rxdesc(hw, (u8 *)entry,
+			if (!_rtl_pci_init_one_rxdesc(hw, NULL, (u8 *)entry,
 						      rxring_idx, i))
 				return -ENOMEM;
 		}

^ permalink raw reply

* Re: [PATCH V3 for 3.19]  rtlwifi: Fix error when accessing unmapped memory in skb
From: Eric Biggers @ 2014-12-31  5:07 UTC (permalink / raw)
  To: Larry Finger; +Cc: kvalo, linux-wireless, netdev, Stable
In-Reply-To: <1419996787-17395-1-git-send-email-Larry.Finger@lwfinger.net>

On Tue, Dec 30, 2014 at 09:33:07PM -0600, Larry Finger wrote:
> v3 - Unmap skb before trying to allocate a new one so as to not leak mapping.

Looks good to me, although I'm not sure about the handling of DMA mapping errors
(perhaps that's something that drivers typically don't even try to handle?).
Anyway, the skb allocation issue appears to be resolved now.  I am running your
patch with an extra hack to inject some occasional skb allocation failures, and
I haven't noticed any problems except dropped packets.

Eric

^ permalink raw reply

* [PATCH net-next] net: More vlan tests before registering netdevice
From: Yuval Mintz @ 2014-12-31  6:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, Yuval Mintz

When register_netdevice() is called, netdevice's vlan filtering feature
and supplied callbacks are checked to see the vlan implementation is
not buggy.
This adds an additional test - see that the vlan_features were filled
correctly, as the vlan devices inherits those as its own features;
Incorrect values set there would later prevent the vlan interface from being
registered itself [as it doesn't implement the filtering ndos].

Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
---
Hi Dave,

Not sure why take such a defensive approach regarding this feature.
Perhaps it would have been better to simply remove these checks altogether.

Cheers,
Yuval
---

 net/core/dev.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3f191da..8a663b2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6253,10 +6253,11 @@ int register_netdevice(struct net_device *dev)
 		}
 	}
 
-	if (((dev->hw_features | dev->features) &
-	     NETIF_F_HW_VLAN_CTAG_FILTER) &&
-	    (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
-	     !dev->netdev_ops->ndo_vlan_rx_kill_vid)) {
+	if ((((dev->hw_features | dev->features) &
+	      NETIF_F_HW_VLAN_CTAG_FILTER) &&
+	     (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
+	      !dev->netdev_ops->ndo_vlan_rx_kill_vid)) ||
+	    (dev->vlan_features & NETIF_F_HW_VLAN_CTAG_FILTER) {
 		netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
 		ret = -EINVAL;
 		goto err_uninit;
-- 
1.9.3

^ permalink raw reply related

* Re: Fw: [Bug 82471] New: net/core/dev.c skb_war_bad_offload
From: Richard Laager @ 2014-12-31  6:51 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1418805852.5277.25.camel@watermelon.coderich.net>

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

I was able to do some bisection using Ubuntu-packaged kernels.

The kernel from Precise (3.2.0-74.109) works (on a Trusty system).

On 3.5.0-51-generic (3.5.0-51.76) from Quantal, I get a different kind
of brokenness. I don't get a stack dump, but I get this kernel message
printed:
skbuff: bond0.7: received packets cannot be forwarded while LRO is enabled

The kernels from Raring (3.8.0-35.50) and Saucy (3.11.0-26.45) are
broken in the same way as Trusty.

Does that skbuff error message provide any clue? If not, it seems that
I'll have to bisect from working-on-3.2 to broken-on-3.5, then revert
that offending commit in each test as I bisect again from 3.5 to 3.8?

-- 
Richard

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

^ permalink raw reply

* Possible BUG in ipv6_find_hdr function for fragmented packets
From: Rahul Sharma @ 2014-12-31  7:03 UTC (permalink / raw)
  To: netdev

Hello netdev,

I have observed a problem when I added an AH header before protocol
header (OSPFv3) while implementing authentication support for OSPFv3.

Problem: Fragmented packets which include authentication header don't
get reassembled in the kernel. This was because ipv6_find_hdr returns
ENOENT for the non-first fragment since AH is an extension header.

Firstly, this comment  "Note that non-1st fragment is special case
that "the protocol number of last header" is "next header" field in
Fragment header" ('last header' doesn't include AH or other extension
headers) before ipv6_find_hdr looks incorrect as per the description
of the fragmentation process in RFC2460. The rfc clearly states that
next header value in the fragments will be the first header of the
Fragmentable part of the original packet which could be AH (51) as in
our case.

This code looks like a problem:
if (_frag_off) {
253                                 if (target < 0 &&
254                                     ((!ipv6_ext_hdr(hp->nexthdr)) ||
255                                      hp->nexthdr == NEXTHDR_NONE)) {
256                                         if (fragoff)
257                                                 *fragoff = _frag_off;
258                                         return hp->nexthdr;
259                                 }
260                                 return -ENOENT;
261                         }

For non-first fragments, the 'next header' in the fragment header
would *always* be AUTH (or whatever extension header is the first
header in first fragment). But the above code will keep on returning
ENOENT for the non-first fragment in such cases.

Solution: I suggest we should get away with this check
((!ipv6_ext_hdr(hp->nexthdr)) ||hp->nexthdr == NEXTHDR_NONE))  and
simply return hp->nexthdr if the _frag_off is non zero. I tested it on
my machine and it works. Adding an special case for NEXTHDR_AUTH also
works for me.

Thanks,
Rahul

^ permalink raw reply

* pull request: bluetooth-next 2014-12-31
From: Johan Hedberg @ 2014-12-31  7:25 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-bluetooth

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

Hi Dave,

Here's the first batch of bluetooth patches for 3.20.

 - Cleanups & fixes to ieee802154  drivers
 - Fix synchronization of mgmt commands with respective HCI commands
 - Add self-tests for LE pairing crypto functionality
 - Remove 'BlueFritz!' specific handling from core using a new quirk flag
 - Public address configuration support for ath3012
 - Refactor debugfs support into a dedicated file
 - Initial support for LE Data Length Extension feature from Bluetooth 4.2

Please let me know if there are any issues pulling. Thanks.

Johan

---
The following changes since commit 86c8fc4bbe14b8950e62d379bb57722427ad3d67:

  Merge tag 'mac80211-for-davem-2014-12-18' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211 (2014-12-18 15:33:49 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git for-upstream

for you to fetch changes up to e64b4fb66cc428cef0a62ea899b671c1aad05f3a:

  Bluetooth: Add timing information to ECDH test case runs (2014-12-30 10:32:11 +0200)

----------------------------------------------------------------
Alexander Aring (11):
      nl802154: introduce cca mode enums
      ieee802154: rework cca setting
      nl802154: introduce support for cca settings
      at86rf230: add reset state cca handling
      at86rf230: remove if branch
      at86rf230: make at86rf230_async_error inline
      at86rf230: fix context pointer handling
      at86rf230: remove unnecessary assign
      at86rf230: cleanup check on trac status
      mac802154: iface: check concurrent ifaces
      ieee802154: iface: move multiple node type check

Andrey Yurovsky (2):
      at86rf230: fix register read for part version
      at86rf230: remove version check for AT86RF212

Johan Hedberg (12):
      Bluetooth: Split hci_update_page_scan into two functions
      Bluetooth: Split hci_request helpers to hci_request.[ch]
      Bluetooth: Add hci_request support for hci_update_background_scan
      Bluetooth: Fix Remove Device to wait for HCI before sending cmd_complete
      Bluetooth: Fix Add Device to wait for HCI before sending cmd_complete
      Bluetooth: Add return parameter to cmd_complete callbacks
      Bluetooth: Move hci_update_page_scan to hci_request.c
      Bluetooth: Fix const declarations for smp_f5 and smp_f6
      Bluetooth: Add support for ECDH test cases
      Bluetooth: Add skeleton for SMP self-tests
      Bluetooth: Add legacy SMP tests
      Bluetooth: Add LE Secure Connections tests for SMP

Jukka Rissanen (1):
      Bluetooth: 6lowpan: Add IPSP PSM value

Marcel Holtmann (17):
      Bluetooth: Support static address when BR/EDR has been disabled
      Bluetooth: Add skeleton functions for debugfs creation
      Bluetooth: Move common debugfs file creation into hci_debugfs.c
      Bluetooth: Move BR/EDR debugfs file creation into hci_debugfs.c
      Bluetooth: Move LE debugfs file creation into hci_debugfs.c
      Bluetooth: Add structures for LE Data Length Extension feature
      Bluetooth: Enable basics for LE Data Length Extension feature
      Bluetooth: Store default and maximum LE data length settings
      Bluetooth: Create debugfs directory for each connection handle
      Bluetooth: Remove duplicate constant for RFCOMM PSM
      Bluetooth: Introduce HCI_QUIRK_BROKEN_LOCAL_COMMANDS constant
      Bluetooth: bfusb: Set the HCI_QUIRK_BROKEN_LOCAL_COMMANDS quirk
      Bluetooth: btusb: Set the HCI_QUIRK_BROKEN_LOCAL_COMMANDS quirk
      Bluetooth: Remove BlueFritz! specific check from initialization
      Bluetooth: Add support for self testing framework
      Bluetooth: Add timing information to SMP test case runs
      Bluetooth: Add timing information to ECDH test case runs

Stefan Schmidt (6):
      ieee802154/at86rf230: Remove unneeded blank lines
      ieee802154/at86rf230: Align to opening parenthesis
      ieee802154/at86rf230: Fix typo unkown -> unknown
      ieee802154/cc2520: Remove extra blank lines
      ieee802154/mrf24j40: Fix typo begining -> beginning
      ieee802154/mrf24j40: Fix alignment of parenthesis

Toshi Kikuchi (1):
      Bluetooth: btusb: support public address configuration for ath3012

Varka Bhadram (3):
      cc2520: use devm_kzalloc(.., sizeof(*pointer), ..) pattern
      cc2520: remove 'ret' goto label
      cc2520: fix zero perm_extended_addr address

Wolfram Sang (1):
      net: ieee802154: don't use devm_pinctrl_get_select_default() in probe

 drivers/bluetooth/bfusb.c          |    2 +
 drivers/bluetooth/btusb.c          |   34 +-
 drivers/net/ieee802154/at86rf230.c |   80 +-
 drivers/net/ieee802154/cc2520.c    |   27 +-
 drivers/net/ieee802154/mrf24j40.c  |    6 +-
 include/net/bluetooth/hci.h        |   53 ++
 include/net/bluetooth/hci_core.h   |   35 +-
 include/net/bluetooth/l2cap.h      |    1 +
 include/net/bluetooth/rfcomm.h     |    2 -
 include/net/cfg802154.h            |   10 +-
 include/net/ieee802154_netdev.h    |    4 +-
 include/net/mac802154.h            |    5 +-
 include/net/nl802154.h             |   45 +-
 net/bluetooth/Kconfig              |   27 +
 net/bluetooth/Makefile             |    4 +-
 net/bluetooth/af_bluetooth.c       |    6 +
 net/bluetooth/hci_conn.c           |    4 +
 net/bluetooth/hci_core.c           | 1640 +----------------------------------
 net/bluetooth/hci_debugfs.c        | 1076 +++++++++++++++++++++++
 net/bluetooth/hci_debugfs.h        |   26 +
 net/bluetooth/hci_event.c          |   71 +-
 net/bluetooth/hci_request.c        |  555 ++++++++++++
 net/bluetooth/hci_request.h        |   54 ++
 net/bluetooth/mgmt.c               |  265 ++++--
 net/bluetooth/rfcomm/core.c        |    4 +-
 net/bluetooth/selftest.c           |  244 ++++++
 net/bluetooth/selftest.h           |   45 +
 net/bluetooth/smp.c                |  335 ++++++-
 net/bluetooth/smp.h                |   13 +
 net/ieee802154/nl-mac.c            |    4 +-
 net/ieee802154/nl802154.c          |   46 +-
 net/ieee802154/rdev-ops.h          |    7 +
 net/ieee802154/sysfs.c             |    2 +-
 net/mac802154/cfg.c                |   21 +
 net/mac802154/driver-ops.h         |    5 +-
 net/mac802154/iface.c              |  100 ++-
 net/mac802154/mac_cmd.c            |    6 +-
 37 files changed, 3084 insertions(+), 1780 deletions(-)
 create mode 100644 net/bluetooth/hci_debugfs.c
 create mode 100644 net/bluetooth/hci_debugfs.h
 create mode 100644 net/bluetooth/hci_request.c
 create mode 100644 net/bluetooth/hci_request.h
 create mode 100644 net/bluetooth/selftest.c
 create mode 100644 net/bluetooth/selftest.h


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [Question] What's the noop_qdisc introduced for in the kernel?
From: Dennis Chen @ 2014-12-31  7:31 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev
In-Reply-To: <CAHA+R7O_ypxh4DjQ1eeKZyGFpYbKQAgKkEjrmk400Ud+zLeEow@mail.gmail.com>

On Wed, Dec 31, 2014 at 4:15 AM, Cong Wang <cwang@twopensource.com> wrote:
> On Tue, Dec 30, 2014 at 1:23 AM, Dennis Chen <kernel.org.gnu@gmail.com> wrote:
>> After google and the code reading, seems this Qdisc instance is only
>> used for the initialization purpose, I can't find the reason that this
>> object introduced in the kernel. Does anybody know what the historical
>> reason is for this invention? the purpose or the benefit for this
>> Qdisc object?
>>
>
> Not just for initialization, it is kinda a null qdisc when
> the previous qdisc gets removed:
>
>         /* ... and graft new one */
>         if (qdisc == NULL)
>                 qdisc = &noop_qdisc;
>
> or the entire device is not activated yet. It guarantees no
> packets can be sent out via this qdisc.

Thanks Cong, got it. The device will use the noop_qdisc when it's not
activated yet...

BTW, do you have some methods to recommend if I want to find the
initial codes that introduce the noop_qdisc first in the kernel? I
tried to search the LKML archives and google, but seems it's not easy
to do that

-- 
Den

^ permalink raw reply

* [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Fu, Zhonghui @ 2014-12-31  8:20 UTC (permalink / raw)
  To: brudley, Arend van Spriel, Franky Lin, meuleman, kvalo, linville,
	pieterpg, hdegoede, wens, linux-wireless, brcm80211-dev-list,
	netdev, linux-kernel

>From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger
twice suspend/resume operations for one WiFi chip to do
the same things. This patch avoid this case.

Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 3c06e93..eee7818 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
 static int brcmf_ops_sdio_suspend(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
 	mmc_pm_flag_t sdio_flags;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
 
+	if (func->num == 2) {
+		return 0;
+	}
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	atomic_set(&sdiodev->suspend, true);
 
 	if (sdiodev->wowl_enabled) {
@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 static int brcmf_ops_sdio_resume(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
+
+	if (func->num == 2) {
+		return 0;
+	}
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	if (sdiodev->pdata->oob_irq_supported)
 		disable_irq_wake(sdiodev->pdata->oob_irq_nr);
 	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1

^ permalink raw reply related

* [PATCH] wait_for_completion_timeout does not return < 0
From: Nicholas Mc Guire @ 2014-12-31  9:04 UTC (permalink / raw)
  To: Inaky Perez-Gonzalez; +Cc: linux-wimax, netdev, linux-kernel, Nicholas Mc Guire

This is only removing the comment which is misleading as
wait_for_completion_timeout does not return < 0 thus there
never is anything to be passed on.

patch is against linux-next 3.19.0-rc1 -next-20141226

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 drivers/net/wimax/i2400m/driver.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 9c78090..0a6384e 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -197,7 +197,6 @@ int i2400m_op_reset(struct wimax_dev *wimax_dev)
 		result = -ETIMEDOUT;
 	else if (result > 0)
 		result = ctx.result;
-	/* if result < 0, pass it on */
 	mutex_lock(&i2400m->init_mutex);
 	i2400m->reset_ctx = NULL;
 	mutex_unlock(&i2400m->init_mutex);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net-next 1/2] net: Add Transparent Ethernet Bridging GRO support.
From: Or Gerlitz @ 2014-12-31  9:19 UTC (permalink / raw)
  To: Jesse Gross; +Cc: David Miller, Linux Netdev List
In-Reply-To: <1419995416-29987-1-git-send-email-jesse@nicira.com>

On Wed, Dec 31, 2014 at 5:10 AM, Jesse Gross <jesse@nicira.com> wrote:
> Currently the only tunnel protocol that supports GRO with encapsulated
> Ethernet is VXLAN. This pulls out the Ethernet code into a proper layer
> so that it can be used by other tunnel protocols such as GRE and Geneve.

Hi Jesse,

Thanks for taking care of that, I also had it coded under the
intention of adding GRO support for OVS's TEB based GRE, but didn't
make it to submit before your post... anyway, I would recommend that
you break this patch into two:

1. basic TEB GRO support
2. refactoring of the VXLAN GRO logic to use it

Or.

>
> Signed-off-by: Jesse Gross <jesse@nicira.com>
> ---
>  drivers/net/vxlan.c         | 53 +++-----------------------
>  include/linux/etherdevice.h |  4 ++
>  net/ethernet/eth.c          | 92 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 7fbd89f..2ab0922 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -549,10 +549,7 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
>  {
>         struct sk_buff *p, **pp = NULL;
>         struct vxlanhdr *vh, *vh2;
> -       struct ethhdr *eh, *eh2;
> -       unsigned int hlen, off_vx, off_eth;
> -       const struct packet_offload *ptype;
> -       __be16 type;
> +       unsigned int hlen, off_vx;
>         int flush = 1;
>
>         off_vx = skb_gro_offset(skb);
> @@ -563,17 +560,6 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
>                 if (unlikely(!vh))
>                         goto out;
>         }
> -       skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
> -       skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
> -
> -       off_eth = skb_gro_offset(skb);
> -       hlen = off_eth + sizeof(*eh);
> -       eh   = skb_gro_header_fast(skb, off_eth);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               eh = skb_gro_header_slow(skb, hlen, off_eth);
> -               if (unlikely(!eh))
> -                       goto out;
> -       }
>
>         flush = 0;
>
> @@ -582,28 +568,16 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head, struct sk_buff
>                         continue;
>
>                 vh2 = (struct vxlanhdr *)(p->data + off_vx);
> -               eh2 = (struct ethhdr   *)(p->data + off_eth);
> -               if (vh->vx_vni != vh2->vx_vni || compare_ether_header(eh, eh2)) {
> +               if (vh->vx_vni != vh2->vx_vni) {
>                         NAPI_GRO_CB(p)->same_flow = 0;
>                         continue;
>                 }
>         }
>
> -       type = eh->h_proto;
> -
> -       rcu_read_lock();
> -       ptype = gro_find_receive_by_type(type);
> -       if (ptype == NULL) {
> -               flush = 1;
> -               goto out_unlock;
> -       }
> -
> -       skb_gro_pull(skb, sizeof(*eh)); /* pull inner eth header */
> -       skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
> -       pp = ptype->callbacks.gro_receive(head, skb);
> +       skb_gro_pull(skb, sizeof(struct vxlanhdr));
> +       skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
> +       pp = eth_gro_receive(head, skb);
>
> -out_unlock:
> -       rcu_read_unlock();
>  out:
>         NAPI_GRO_CB(skb)->flush |= flush;
>
> @@ -612,24 +586,9 @@ out:
>
>  static int vxlan_gro_complete(struct sk_buff *skb, int nhoff)
>  {
> -       struct ethhdr *eh;
> -       struct packet_offload *ptype;
> -       __be16 type;
> -       int vxlan_len  = sizeof(struct vxlanhdr) + sizeof(struct ethhdr);
> -       int err = -ENOSYS;
> -
>         udp_tunnel_gro_complete(skb, nhoff);
>
> -       eh = (struct ethhdr *)(skb->data + nhoff + sizeof(struct vxlanhdr));
> -       type = eh->h_proto;
> -
> -       rcu_read_lock();
> -       ptype = gro_find_complete_by_type(type);
> -       if (ptype != NULL)
> -               err = ptype->callbacks.gro_complete(skb, nhoff + vxlan_len);
> -
> -       rcu_read_unlock();
> -       return err;
> +       return eth_gro_complete(skb, nhoff + sizeof(struct vxlanhdr));
>  }
>
>  /* Notify netdevs that UDP port started listening */
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 41c891d..1d869d1 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -52,6 +52,10 @@ struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
>  #define alloc_etherdev(sizeof_priv) alloc_etherdev_mq(sizeof_priv, 1)
>  #define alloc_etherdev_mq(sizeof_priv, count) alloc_etherdev_mqs(sizeof_priv, count, count)
>
> +struct sk_buff **eth_gro_receive(struct sk_buff **head,
> +                                struct sk_buff *skb);
> +int eth_gro_complete(struct sk_buff *skb, int nhoff);
> +
>  /* Reserved Ethernet Addresses per IEEE 802.1Q */
>  static const u8 eth_reserved_addr_base[ETH_ALEN] __aligned(2) =
>  { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 33a140e..238f38d 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -424,3 +424,95 @@ ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len)
>         return scnprintf(buf, PAGE_SIZE, "%*phC\n", len, addr);
>  }
>  EXPORT_SYMBOL(sysfs_format_mac);
> +
> +struct sk_buff **eth_gro_receive(struct sk_buff **head,
> +                                struct sk_buff *skb)
> +{
> +       struct sk_buff *p, **pp = NULL;
> +       struct ethhdr *eh, *eh2;
> +       unsigned int hlen, off_eth;
> +       const struct packet_offload *ptype;
> +       __be16 type;
> +       int flush = 1;
> +
> +       off_eth = skb_gro_offset(skb);
> +       hlen = off_eth + sizeof(*eh);
> +       eh = skb_gro_header_fast(skb, off_eth);
> +       if (skb_gro_header_hard(skb, hlen)) {
> +               eh = skb_gro_header_slow(skb, hlen, off_eth);
> +               if (unlikely(!eh))
> +                       goto out;
> +       }
> +
> +       flush = 0;
> +
> +       for (p = *head; p; p = p->next) {
> +               if (!NAPI_GRO_CB(p)->same_flow)
> +                       continue;
> +
> +               eh2 = (struct ethhdr *)(p->data + off_eth);
> +               if (compare_ether_header(eh, eh2)) {
> +                       NAPI_GRO_CB(p)->same_flow = 0;
> +                       continue;
> +               }
> +       }
> +
> +       type = eh->h_proto;
> +
> +       rcu_read_lock();
> +       ptype = gro_find_receive_by_type(type);
> +       if (ptype == NULL) {
> +               flush = 1;
> +               goto out_unlock;
> +       }
> +
> +       skb_gro_pull(skb, sizeof(*eh));
> +       skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
> +       pp = ptype->callbacks.gro_receive(head, skb);
> +
> +out_unlock:
> +       rcu_read_unlock();
> +out:
> +       NAPI_GRO_CB(skb)->flush |= flush;
> +
> +       return pp;
> +}
> +EXPORT_SYMBOL(eth_gro_receive);
> +
> +int eth_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> +       struct ethhdr *eh = (struct ethhdr *)(skb->data + nhoff);
> +       __be16 type = eh->h_proto;
> +       struct packet_offload *ptype;
> +       int err = -ENOSYS;
> +
> +       if (skb->encapsulation)
> +               skb_set_inner_mac_header(skb, nhoff);
> +
> +       rcu_read_lock();
> +       ptype = gro_find_complete_by_type(type);
> +       if (ptype != NULL)
> +               err = ptype->callbacks.gro_complete(skb, nhoff +
> +                                                   sizeof(struct ethhdr));
> +
> +       rcu_read_unlock();
> +       return err;
> +}
> +EXPORT_SYMBOL(eth_gro_complete);
> +
> +static struct packet_offload eth_packet_offload __read_mostly = {
> +       .type = cpu_to_be16(ETH_P_TEB),
> +       .callbacks = {
> +               .gro_receive = eth_gro_receive,
> +               .gro_complete = eth_gro_complete,
> +       },
> +};
> +
> +static int __init eth_offload_init(void)
> +{
> +       dev_add_offload(&eth_packet_offload);
> +
> +       return 0;
> +}
> +
> +fs_initcall(eth_offload_init);
> --
> 1.9.1
>
> --
> 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

* [PATCH net-next v1 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Vlad Zolotarov @ 2014-12-31  9:51 UTC (permalink / raw)
  To: netdev; +Cc: gleb, avi, jeffrey.t.kirsher, Vlad Zolotarov

Add the ethtool ops to VF driver to allow querying the RSS indirection table
and RSS Random Key.
 
 - PF driver: Add new VF-PF channel commands.
 - VF driver: Utilize these new commands and add the corresponding
              ethtool callbacks.

New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case.
     More specifically: in cases where the newly added API version is the only one
     allowed. We may consider using a "switch-case" back again when the list of
     allowed API versions in these specific places grows up.

Vlad Zolotarov (5):
  ixgbe: Add a RETA query command to VF-PF channel API
  ixgbevf: Add a RETA query code
  ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
  ixgbevf: Add RSS Key query code
  ixgbevf: Add the appropriate ethtool ops to query RSS indirection
    table and key

 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |   8 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  52 ++++++++++
 drivers/net/ethernet/intel/ixgbevf/ethtool.c      |  37 +++++++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   4 +-
 drivers/net/ethernet/intel/ixgbevf/mbx.h          |   8 ++
 drivers/net/ethernet/intel/ixgbevf/vf.c           | 117 ++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/vf.h           |   2 +
 7 files changed, 227 insertions(+), 1 deletion(-)

-- 
2.1.0

^ permalink raw reply

* [PATCH net-next v1 1/5] ixgbe: Add a RETA query command to VF-PF channel API
From: Vlad Zolotarov @ 2014-12-31  9:51 UTC (permalink / raw)
  To: netdev; +Cc: gleb, avi, jeffrey.t.kirsher, Vlad Zolotarov
In-Reply-To: <1420019519-18139-1-git-send-email-vladz@cloudius-systems.com>

82599 VFs and PF share the same RSS redirection table (RETA). Therefore we
just return it for all VFs.

RETA table is an array of 32 registers and the maximum number of registers
that may be delivered in a single VF-PF channel command is 15. Therefore
we will deliver the whole table in 3 steps: 12, 12 and 8 registers in each
step correspondingly. Thus this patch does the following:

  - Adds a new API version (to specify a new commands set).
  - Adds the IXGBE_VF_GET_RETA_[0,1,2] commands to the VF-PF commands set.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbe_get_vf_reta()).
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  6 +++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 31 ++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index a5cb755..c1123d9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
 	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
 	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
 	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
+	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
 	/* This value should always be last */
 	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
 };
@@ -91,6 +92,11 @@ enum ixgbe_pfvf_api_rev {
 /* mailbox API, version 1.1 VF requests */
 #define IXGBE_VF_GET_QUEUES	0x09 /* get queue configuration */
 
+/* mailbox API, version 1.2 VF requests */
+#define IXGBE_VF_GET_RETA_0	0x0a /* get RETA[0..11]  */
+#define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
+#define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index c76ba90..b1625c8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -427,6 +427,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 #endif /* CONFIG_FCOE */
 		switch (adapter->vfinfo[vf].vf_api) {
 		case ixgbe_mbox_api_11:
+		case ixgbe_mbox_api_12:
 			/*
 			 * Version 1.1 supports jumbo frames on VFs if PF has
 			 * jumbo frames enabled which means legacy VFs are
@@ -894,6 +895,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
 	switch (api) {
 	case ixgbe_mbox_api_10:
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		adapter->vfinfo[vf].vf_api = api;
 		return 0;
 	default:
@@ -917,6 +919,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
 	switch (adapter->vfinfo[vf].vf_api) {
 	case ixgbe_mbox_api_20:
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		break;
 	default:
 		return -1;
@@ -944,6 +947,25 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
+			     u32 *msgbuf, u32 vf, int reta_offset_dw,
+			     size_t dwords)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	int i;
+	u32 *reta = &msgbuf[1];
+
+	/* verify the PF is supporting the correct API */
+	if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	/* Read the appropriate portion of RETA */
+	for (i = 0; i < dwords; i++)
+		reta[i] = IXGBE_READ_REG(hw, IXGBE_RETA(i + reta_offset_dw));
+
+	return 0;
+}
+
 static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 {
 	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1000,6 +1022,15 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 	case IXGBE_VF_GET_QUEUES:
 		retval = ixgbe_get_vf_queues(adapter, msgbuf, vf);
 		break;
+	case IXGBE_VF_GET_RETA_0:
+		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 0, 12);
+		break;
+	case IXGBE_VF_GET_RETA_1:
+		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 12, 12);
+		break;
+	case IXGBE_VF_GET_RETA_2:
+		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 24, 8);
+		break;
 	default:
 		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
 		retval = IXGBE_ERR_MBX;
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v1 2/5] ixgbevf: Add a RETA query code
From: Vlad Zolotarov @ 2014-12-31  9:51 UTC (permalink / raw)
  To: netdev; +Cc: gleb, avi, jeffrey.t.kirsher, Vlad Zolotarov
In-Reply-To: <1420019519-18139-1-git-send-email-vladz@cloudius-systems.com>

   - Added a new API version support.
   - Added the query implementation in the ixgbevf.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbevf_get_reta()).
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  4 +-
 drivers/net/ethernet/intel/ixgbevf/mbx.h          |  6 ++
 drivers/net/ethernet/intel/ixgbevf/vf.c           | 73 +++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/vf.h           |  1 +
 4 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 62a0d8e..ba6ab61 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1880,7 +1880,8 @@ static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
 static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
-	int api[] = { ixgbe_mbox_api_11,
+	int api[] = { ixgbe_mbox_api_12,
+		      ixgbe_mbox_api_11,
 		      ixgbe_mbox_api_10,
 		      ixgbe_mbox_api_unknown };
 	int err = 0, idx = 0;
@@ -3525,6 +3526,7 @@ static int ixgbevf_change_mtu(struct net_device *netdev, int new_mtu)
 
 	switch (adapter->hw.api_version) {
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		max_possible_frame = IXGBE_MAX_JUMBO_FRAME_SIZE;
 		break;
 	default:
diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
index 0bc3005..3e148a8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
@@ -86,6 +86,7 @@ enum ixgbe_pfvf_api_rev {
 	ixgbe_mbox_api_10,	/* API version 1.0, linux/freebsd VF driver */
 	ixgbe_mbox_api_20,	/* API version 2.0, solaris Phase1 VF driver */
 	ixgbe_mbox_api_11,	/* API version 1.1, linux/freebsd VF driver */
+	ixgbe_mbox_api_12,	/* API version 1.2, linux/freebsd VF driver */
 	/* This value should always be last */
 	ixgbe_mbox_api_unknown,	/* indicates that API version is not known */
 };
@@ -104,6 +105,11 @@ enum ixgbe_pfvf_api_rev {
 /* mailbox API, version 1.1 VF requests */
 #define IXGBE_VF_GET_QUEUE	0x09 /* get queue configuration */
 
+/* mailbox API, version 1.2 VF requests */
+#define IXGBE_VF_GET_RETA_0	0x0a /* get RETA[0..11]  */
+#define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
+#define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index cdb53be..8b98cdf 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -258,6 +258,78 @@ static s32 ixgbevf_set_uc_addr_vf(struct ixgbe_hw *hw, u32 index, u8 *addr)
 	return ret_val;
 }
 
+static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
+				    u32 *reta, u32 op, int reta_offset_dw,
+				    size_t dwords)
+{
+	int err;
+
+	msgbuf[0] = op;
+	err = hw->mbx.ops.write_posted(hw, msgbuf, 1);
+
+	if (err)
+		return err;
+
+	err = hw->mbx.ops.read_posted(hw, msgbuf, 1 + dwords);
+
+	if (err)
+		return err;
+
+	msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS;
+
+	/* If we didn't get an ACK there must have been
+	 * some sort of mailbox error so we should treat it
+	 * as such.
+	 */
+	if (msgbuf[0] != (op | IXGBE_VT_MSGTYPE_ACK))
+		return IXGBE_ERR_MBX;
+
+	memcpy(reta + reta_offset_dw, msgbuf + 1, 4 * dwords);
+
+	return 0;
+}
+
+/**
+ * ixgbevf_get_reta - get the RSS redirection table (RETA) contents.
+ * @hw: pointer to the HW structure
+ * @reta: buffer to fill with RETA contents.
+ *
+ * The "reta" buffer should be big enough to contain 32 registers.
+ *
+ * Returns: 0 on success.
+ *          if API doesn't support this operation - (-EPERM).
+ */
+int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta)
+{
+	int err;
+	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
+
+	/* Return an error if API doesn't RETA querying. */
+	if (hw->api_version != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	/* Fetch RETA from the PF. We do it in 3 steps due to mailbox size
+	 * limitation - we can bring up to 15 dwords every time while RETA
+	 * consists of 32 dwords. Therefore we'll bring 12, 12 and 8 dwords in
+	 * each step correspondingly.
+	 */
+
+	/* RETA[0..11] */
+	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_0, 0, 12);
+	if (err)
+		return err;
+
+	/* RETA[12..23] */
+	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_1, 12, 12);
+	if (err)
+		return err;
+
+	/* RETA[24..31] */
+	err = _ixgbevf_get_reta(hw, msgbuf, reta, IXGBE_VF_GET_RETA_2, 24, 8);
+
+	return err;
+}
+
 /**
  *  ixgbevf_set_rar_vf - set device MAC address
  *  @hw: pointer to hardware structure
@@ -545,6 +617,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 	/* do nothing if API doesn't support ixgbevf_get_queues */
 	switch (hw->api_version) {
 	case ixgbe_mbox_api_11:
+	case ixgbe_mbox_api_12:
 		break;
 	default:
 		return 0;
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index 5b17242..73c1b33 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -208,5 +208,6 @@ void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size);
 int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
 int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 		       unsigned int *default_tc);
+int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta);
 #endif /* __IXGBE_VF_H__ */
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v1 3/5] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
From: Vlad Zolotarov @ 2014-12-31  9:51 UTC (permalink / raw)
  To: netdev; +Cc: gleb, avi, jeffrey.t.kirsher, Vlad Zolotarov
In-Reply-To: <1420019519-18139-1-git-send-email-vladz@cloudius-systems.com>

82599 VFs and PF share the same RSS Key. Therefore we will return
the same RSS key for all VFs.

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbe_get_vf_rss_key()).
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h   |  2 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index c1123d9..52e775b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -97,6 +97,8 @@ enum ixgbe_pfvf_api_rev {
 #define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
 #define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
 
+#define IXGBE_VF_GET_RSS_KEY	0x0d /* get RSS key */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index b1625c8..d79415e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -966,6 +966,24 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
+				u32 *msgbuf, u32 vf)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	int i;
+	u32 *rss_key = &msgbuf[1];
+
+	/* verify the PF is supporting the correct API */
+	if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	/* Read the RSS KEY */
+	for (i = 0; i < 10; i++)
+		rss_key[i] = IXGBE_READ_REG(hw, IXGBE_RSSRK(i));
+
+	return 0;
+}
+
 static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 {
 	u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1031,6 +1049,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
 	case IXGBE_VF_GET_RETA_2:
 		retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 24, 8);
 		break;
+	case IXGBE_VF_GET_RSS_KEY:
+		retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
+		break;
 	default:
 		e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
 		retval = IXGBE_ERR_MBX;
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v1 4/5] ixgbevf: Add RSS Key query code
From: Vlad Zolotarov @ 2014-12-31  9:51 UTC (permalink / raw)
  To: netdev; +Cc: gleb, avi, jeffrey.t.kirsher, Vlad Zolotarov
In-Reply-To: <1420019519-18139-1-git-send-email-vladz@cloudius-systems.com>

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
New in v1 (compared to RFC):
   - Use "if-else" statement instead of a "switch-case" for a single option case
     (in ixgbevf_get_rss_key()).

---
 drivers/net/ethernet/intel/ixgbevf/mbx.h |  2 ++
 drivers/net/ethernet/intel/ixgbevf/vf.c  | 44 ++++++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/vf.h  |  1 +
 3 files changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
index 3e148a8..9674ac8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
@@ -110,6 +110,8 @@ enum ixgbe_pfvf_api_rev {
 #define IXGBE_VF_GET_RETA_1	0x0b /* get RETA[12..23] */
 #define IXGBE_VF_GET_RETA_2	0x0c /* get RETA[24..31] */
 
+#define IXGBE_VF_GET_RSS_KEY	0x0d /* get RSS hash key */
+
 /* GET_QUEUES return data indices within the mailbox */
 #define IXGBE_VF_TX_QUEUES	1	/* number of Tx queues supported */
 #define IXGBE_VF_RX_QUEUES	2	/* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 8b98cdf..dbac264 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -290,6 +290,50 @@ static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
 }
 
 /**
+ * ixgbevf_get_rss_key - get the RSS Random Key
+ * @hw: pointer to the HW structure
+ * @reta: buffer to fill with RETA contents.
+ *
+ * The "rss_key" buffer should be big enough to contain 10 registers.
+ *
+ * Returns: 0 on success.
+ *          if API doesn't support this operation - (-EPERM).
+ */
+int ixgbevf_get_rss_key(struct ixgbe_hw *hw, u8 *rss_key)
+{
+	int err;
+	u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
+
+	/* Return and error if API doesn't support RSS Random Key retrieval */
+	if (hw->api_version != ixgbe_mbox_api_12)
+		return -EPERM;
+
+	msgbuf[0] = IXGBE_VF_GET_RSS_KEY;
+	err = hw->mbx.ops.write_posted(hw, msgbuf, 1);
+
+	if (err)
+		return err;
+
+	err = hw->mbx.ops.read_posted(hw, msgbuf, 11);
+
+	if (err)
+		return err;
+
+	msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS;
+
+	/* If we didn't get an ACK there must have been
+	 * some sort of mailbox error so we should treat it
+	 * as such.
+	 */
+	if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK))
+		return IXGBE_ERR_MBX;
+
+	memcpy(rss_key, msgbuf + 1, 40);
+
+	return 0;
+}
+
+/**
  * ixgbevf_get_reta - get the RSS redirection table (RETA) contents.
  * @hw: pointer to the HW structure
  * @reta: buffer to fill with RETA contents.
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index 73c1b33..54f53f2b8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -209,5 +209,6 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
 int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs,
 		       unsigned int *default_tc);
 int ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *reta);
+int ixgbevf_get_rss_key(struct ixgbe_hw *hw, u8 *rss_key);
 #endif /* __IXGBE_VF_H__ */
 
-- 
2.1.0

^ permalink raw reply related

* [PATCH net-next v1 5/5] ixgbevf: Add the appropriate ethtool ops to query RSS indirection table and key
From: Vlad Zolotarov @ 2014-12-31  9:51 UTC (permalink / raw)
  To: netdev; +Cc: gleb, avi, jeffrey.t.kirsher, Vlad Zolotarov
In-Reply-To: <1420019519-18139-1-git-send-email-vladz@cloudius-systems.com>

Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
 drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37 ++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index cc0e5b7..255bbc8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -792,6 +792,40 @@ static int ixgbevf_set_coalesce(struct net_device *netdev,
 	return 0;
 }
 
+static u32 ixgbevf_get_rxfh_indir_size(struct net_device *netdev)
+{
+	return 128;
+}
+
+static u32 ixgbevf_get_rxfh_key_size(struct net_device *netdev)
+{
+	return 40;
+}
+
+static int ixgbevf_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key,
+			    u8 *hfunc)
+{
+	struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+	int err;
+
+	if (hfunc)
+		*hfunc = ETH_RSS_HASH_TOP;
+
+	if (indir) {
+		err = ixgbevf_get_reta(&adapter->hw, indir);
+		if (err)
+			return err;
+	}
+
+	if (key) {
+		err = ixgbevf_get_rss_key(&adapter->hw, key);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops ixgbevf_ethtool_ops = {
 	.get_settings           = ixgbevf_get_settings,
 	.get_drvinfo            = ixgbevf_get_drvinfo,
@@ -809,6 +843,9 @@ static const struct ethtool_ops ixgbevf_ethtool_ops = {
 	.get_ethtool_stats      = ixgbevf_get_ethtool_stats,
 	.get_coalesce           = ixgbevf_get_coalesce,
 	.set_coalesce           = ixgbevf_set_coalesce,
+	.get_rxfh_indir_size    = ixgbevf_get_rxfh_indir_size,
+	.get_rxfh_key_size      = ixgbevf_get_rxfh_key_size,
+	.get_rxfh		= ixgbevf_get_rxfh,
 };
 
 void ixgbevf_set_ethtool_ops(struct net_device *netdev)
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Arend van Spriel @ 2014-12-31  9:56 UTC (permalink / raw)
  To: Fu, Zhonghui
  Cc: brudley, Franky Lin, meuleman, kvalo, linville, pieterpg,
	hdegoede, wens, linux-wireless, brcm80211-dev-list, netdev,
	linux-kernel
In-Reply-To: <54A3B1B4.7090705@linux.intel.com>

On 12/31/14 09:20, Fu, Zhonghui wrote:
>  From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
> From: Zhonghui Fu<zhonghui.fu@linux.intel.com>
> Date: Mon, 29 Dec 2014 21:25:31 +0800
> Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
>
> WiFi chip has 2 SDIO functions, and PM core will trigger
> twice suspend/resume operations for one WiFi chip to do
> the same things. This patch avoid this case.

We have a patch queued up for this as well, but this one looks good 
enough although I personally prefer container_of() instead of 
dev_to_sdio_func().

Acked-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com>
> ---
>   drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
>   1 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index 3c06e93..eee7818 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
>   static int brcmf_ops_sdio_suspend(struct device *dev)
>   {
>   	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> -	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
> +	struct brcmf_sdio_dev *sdiodev;
>   	mmc_pm_flag_t sdio_flags;
> +	struct sdio_func *func = dev_to_sdio_func(dev);
>
>   	brcmf_dbg(SDIO, "Enter\n");
>
> +	if (func->num == 2) {
> +		return 0;
> +	}
> +
> +	sdiodev = bus_if->bus_priv.sdio;
> +
>   	atomic_set(&sdiodev->suspend, true);
>
>   	if (sdiodev->wowl_enabled) {
> @@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
>   static int brcmf_ops_sdio_resume(struct device *dev)
>   {
>   	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> -	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
> +	struct brcmf_sdio_dev *sdiodev;
> +	struct sdio_func *func = dev_to_sdio_func(dev);
>
>   	brcmf_dbg(SDIO, "Enter\n");
> +
> +	if (func->num == 2) {
> +		return 0;
> +	}
> +
> +	sdiodev = bus_if->bus_priv.sdio;
> +
>   	if (sdiodev->pdata->oob_irq_supported)
>   		disable_irq_wake(sdiodev->pdata->oob_irq_nr);
>   	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
> -- 1.7.1
>

^ permalink raw reply

* Re: [PATCH] wait_for_completion_timeout does not return < 0
From: Sudip Mukherjee @ 2014-12-31 10:31 UTC (permalink / raw)
  To: Nicholas Mc Guire; +Cc: Inaky Perez-Gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <1420016643-19707-1-git-send-email-der.herr@hofr.at>

On Wed, Dec 31, 2014 at 04:04:03AM -0500, Nicholas Mc Guire wrote:
> This is only removing the comment which is misleading as
> wait_for_completion_timeout does not return < 0 thus there
> never is anything to be passed on.

a small doubt - 
i am seeing that:
unsigned long wait_for_completion_timeout() is calling
long wait_for_common()  which is again calling 
long __wait_for_common which is again calling
long do_wait_for_common()

now the return value from do_wait_for_common can be -ERESTARTSYS,
so then what happens when wait_for_completion_timeout return this -ERESTARTSYS as an unsigned value ?
it becomes a positive value, and ultimately ctx.result (which is 0) is returned.
so then are we just ignoring the error value from do_wait_for_common() ?

sudip

> 
> patch is against linux-next 3.19.0-rc1 -next-20141226
> 
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> ---
>  drivers/net/wimax/i2400m/driver.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> index 9c78090..0a6384e 100644
> --- a/drivers/net/wimax/i2400m/driver.c
> +++ b/drivers/net/wimax/i2400m/driver.c
> @@ -197,7 +197,6 @@ int i2400m_op_reset(struct wimax_dev *wimax_dev)
>  		result = -ETIMEDOUT;
>  	else if (result > 0)
>  		result = ctx.result;
> -	/* if result < 0, pass it on */
>  	mutex_lock(&i2400m->init_mutex);
>  	i2400m->reset_ctx = NULL;
>  	mutex_unlock(&i2400m->init_mutex);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH] wait_for_completion_timeout does not return < 0
From: Nicholas Mc Guire @ 2014-12-31 10:37 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: Inaky Perez-Gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <20141231103157.GA20842@sudip-PC>

On Wed, 31 Dec 2014, Sudip Mukherjee wrote:

> On Wed, Dec 31, 2014 at 04:04:03AM -0500, Nicholas Mc Guire wrote:
> > This is only removing the comment which is misleading as
> > wait_for_completion_timeout does not return < 0 thus there
> > never is anything to be passed on.
> 
> a small doubt - 
> i am seeing that:
> unsigned long wait_for_completion_timeout() is calling
> long wait_for_common()  which is again calling 
> long __wait_for_common which is again calling
> long do_wait_for_common()
> 
> now the return value from do_wait_for_common can be -ERESTARTSYS,
> so then what happens when wait_for_completion_timeout return this -ERESTARTSYS as an unsigned value ?
> it becomes a positive value, and ultimately ctx.result (which is 0) is returned.
> so then are we just ignoring the error value from do_wait_for_common() ?
>
 
ESTARTSYS only can be returned if state matches in do_wait_for_common
but wait_for_completion_timemout passes TASK_UNINTERRUPTIBLE
so signal_pending_state will return 0 and thus it will never
return -ERESTARTSYS.

my understanding of the callchain is:
wait_for_completion_timemout which is uninterruptible
  -> wait_for_common(...TASK_UNINTERRUPTIBLE)
    -> __wait_for_common(...TASK_UNINTERRUPTIBLE)
      -> do_wait_for_common(...TASK_UNINTERRUPTIBLE)
        -> signal_pending_state(TASK_UNINTERRUPTIBLE...)

static inline int signal_pending_state(long state, struct task_struct *p)
{
        if (!(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
                return 0;

so wait_for_completion_timemout should return >=0 only

thx!
hofrat

> > 
> > patch is against linux-next 3.19.0-rc1 -next-20141226
> > 
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> >  drivers/net/wimax/i2400m/driver.c |    1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> > index 9c78090..0a6384e 100644
> > --- a/drivers/net/wimax/i2400m/driver.c
> > +++ b/drivers/net/wimax/i2400m/driver.c
> > @@ -197,7 +197,6 @@ int i2400m_op_reset(struct wimax_dev *wimax_dev)
> >  		result = -ETIMEDOUT;
> >  	else if (result > 0)
> >  		result = ctx.result;
> > -	/* if result < 0, pass it on */
> >  	mutex_lock(&i2400m->init_mutex);
> >  	i2400m->reset_ctx = NULL;
> >  	mutex_unlock(&i2400m->init_mutex);
> > -- 
> > 1.7.10.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH] wait_for_completion_timeout does not return < 0
From: Sudip Mukherjee @ 2014-12-31 10:50 UTC (permalink / raw)
  To: Nicholas Mc Guire; +Cc: Inaky Perez-Gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <20141231103711.GA30533@opentech.at>

On Wed, Dec 31, 2014 at 11:37:11AM +0100, Nicholas Mc Guire wrote:
> On Wed, 31 Dec 2014, Sudip Mukherjee wrote:
> 
> > On Wed, Dec 31, 2014 at 04:04:03AM -0500, Nicholas Mc Guire wrote:
> > > This is only removing the comment which is misleading as
> > > wait_for_completion_timeout does not return < 0 thus there
> > > never is anything to be passed on.
> > 
> > a small doubt - 
> > i am seeing that:
> > unsigned long wait_for_completion_timeout() is calling
> > long wait_for_common()  which is again calling 
> > long __wait_for_common which is again calling
> > long do_wait_for_common()
> > 
> > now the return value from do_wait_for_common can be -ERESTARTSYS,
> > so then what happens when wait_for_completion_timeout return this -ERESTARTSYS as an unsigned value ?
> > it becomes a positive value, and ultimately ctx.result (which is 0) is returned.
> > so then are we just ignoring the error value from do_wait_for_common() ?
> >
>  
> ESTARTSYS only can be returned if state matches in do_wait_for_common
> but wait_for_completion_timemout passes TASK_UNINTERRUPTIBLE
> so signal_pending_state will return 0 and thus it will never
> return -ERESTARTSYS.
> 
> my understanding of the callchain is:
> wait_for_completion_timemout which is uninterruptible
>   -> wait_for_common(...TASK_UNINTERRUPTIBLE)
>     -> __wait_for_common(...TASK_UNINTERRUPTIBLE)
>       -> do_wait_for_common(...TASK_UNINTERRUPTIBLE)
>         -> signal_pending_state(TASK_UNINTERRUPTIBLE...)
> 
> static inline int signal_pending_state(long state, struct task_struct *p)
> {
>         if (!(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
>                 return 0;
> 
> so wait_for_completion_timemout should return >=0 only

doubt cleared.

thanks
sudip
> 
> thx!
> hofrat
> 
> > > 
> > > patch is against linux-next 3.19.0-rc1 -next-20141226
> > > 
> > > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > > ---
> > >  drivers/net/wimax/i2400m/driver.c |    1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> > > index 9c78090..0a6384e 100644
> > > --- a/drivers/net/wimax/i2400m/driver.c
> > > +++ b/drivers/net/wimax/i2400m/driver.c
> > > @@ -197,7 +197,6 @@ int i2400m_op_reset(struct wimax_dev *wimax_dev)
> > >  		result = -ETIMEDOUT;
> > >  	else if (result > 0)
> > >  		result = ctx.result;
> > > -	/* if result < 0, pass it on */
> > >  	mutex_lock(&i2400m->init_mutex);
> > >  	i2400m->reset_ctx = NULL;
> > >  	mutex_unlock(&i2400m->init_mutex);
> > > -- 
> > > 1.7.10.4
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
From: Sergei Shtylyov @ 2014-12-31 11:22 UTC (permalink / raw)
  To: Fu, Zhonghui, brudley, Arend van Spriel, Franky Lin, meuleman,
	kvalo, linville, pieterpg, hdegoede, wens, linux-wireless,
	brcm80211-dev-list, netdev, linux-kernel
In-Reply-To: <54A3B1B4.7090705@linux.intel.com>

Hello.

On 12/31/2014 11:20 AM, Fu, Zhonghui wrote:

>  From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
> From: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> Date: Mon, 29 Dec 2014 21:25:31 +0800
> Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

> WiFi chip has 2 SDIO functions, and PM core will trigger
> twice suspend/resume operations for one WiFi chip to do
> the same things. This patch avoid this case.

> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> ---
>   drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
>   1 files changed, 17 insertions(+), 2 deletions(-)

> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index 3c06e93..eee7818 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
>   static int brcmf_ops_sdio_suspend(struct device *dev)
>   {
>   	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> -	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
> +	struct brcmf_sdio_dev *sdiodev;
>   	mmc_pm_flag_t sdio_flags;
> +	struct sdio_func *func = dev_to_sdio_func(dev);
>
>   	brcmf_dbg(SDIO, "Enter\n");
>
> +	if (func->num == 2) {
> +		return 0;
> +	}

    {} not needed.

[...]
> @@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
>   static int brcmf_ops_sdio_resume(struct device *dev)
>   {
>   	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> -	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
> +	struct brcmf_sdio_dev *sdiodev;
> +	struct sdio_func *func = dev_to_sdio_func(dev);
>
>   	brcmf_dbg(SDIO, "Enter\n");
> +
> +	if (func->num == 2) {
> +		return 0;
> +	}

    Same here.

[...]

WBR, Sergei

^ permalink raw reply

* Re: [PATCH] can: kvaser_usb: Add support for the Usbcan-II family
From: Olivier Sobrie @ 2014-12-31 12:13 UTC (permalink / raw)
  To: Ahmed S. Darwish
  Cc: Oliver Hartkopp, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20141230153326.GA3798@linux>

On Tue, Dec 30, 2014 at 10:33:26AM -0500, Ahmed S. Darwish wrote:
> On Sun, Dec 28, 2014 at 10:51:34PM +0100, Olivier Sobrie wrote:
> [...]
> > > > >  
> > > > > +	if (LEAF_PRODUCT_ID(id->idProduct)) {
> > > > > +		dev->family = KVASER_LEAF;
> > > > > +		dev->max_channels = LEAF_MAX_NET_DEVICES;
> > > > > +	} else if (USBCAN_PRODUCT_ID(id->idProduct)) {
> > > > > +		dev->family = KVASER_USBCAN;
> > > > > +		dev->max_channels = USBCAN_MAX_NET_DEVICES;
> > > > > +	} else {
> > > > > +		dev_err(&intf->dev, "Product ID (%d) does not belong to any "
> > > > > +				    "known Kvaser USB family", id->idProduct);
> > > > > +		return -ENODEV;
> > > > > +	}
> > > > > +
> > > > 
> > > > Is it really required to keep max_channels in the kvaser_usb structure?
> > > > If I looked correctly, you use this variable as a replacement for
> > > > MAX_NET_DEVICES in the code and MAX_NET_DEVICES is only used in probe
> > > > and disconnect functions. I think it can even be replaced by nchannels
> > > > in the disconnect path. So I also think that it don't need to be in the
> > > > kvaser_usb structure.
> > > > 
> > > 
> > > hmmm.. given the current state of error arbitration explained
> > > above, where I cannot accept a dev->nchannels > 2, I guess we
> > > have two options:
> > > 
> > > a) Remove max_channels, and hardcode the channels count
> > > correctness logic as follows:
> > > 
> > >         dev->nchannels = msg.u.cardinfo.nchannels;
> > >         if ((dev->family == USBCAN && dev->nchannels > USBCAN_MAX_NET_DEVICES)
> > >             || (dev->family == LEAF && dev->nchannels > LEAF_MAX_NET_DEVICES))
> > >                 return -EINVAL
> > > 
> > > b) Leave max_channels in 'struct kvaser_usb' as is.
> > > 
> > > I personally prefer the solution at 'b)' but I can do it as
> > > in 'a)' if you prefer :-)
> > 
> > Keeping max_channels in the kvaser_usb structure is useless because it
> > is only used in one function that is called in the probe function.
> > 
> > I would prefer to have:
> > 	if (dev->nchannels > MAX_NET_DEVICES)
> > 		return -EINVAL
> > 
> > 	if ((dev->family == USBCAN) &&
> > 	    (dev->nchannels > MAX_USBCAN_NET_DEVICES))
> > 		return -EINVAL
> > 
> > You can remove LEAF_MAX_NET_DEVICES which is not used, keep
> > MAX_NET_DEVICES equals to 3 and remove the MAX() macro.
> > The test specific to the USBCAN family can eventually be moved in the
> > kvaser_usb_probe() function.
> > 
> 
> Quite nice, will do it that way in v3.
> 

Thank you.
Please also check your patches with scripts/checkpatch.pl.
I see several warnings when I run it. Please fix them.

All the best for New Year's Eve,

-- 
Olivier

^ 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