Netdev List
 help / color / mirror / Atom feed
* [PATCH net] cxgb4: Don't allocate adapter structure for all PF's
From: Hariprasad Shenai @ 2014-09-15 21:28 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, nirranjan, kumaras, anish, Hariprasad Shenai

commit 35b1de557970 ("rdma/cxgb4: Fixes cxgb4 probe failure in VM when PF is
exposed through PCI Passthrough") moved the code to check for SR-IOV PF[0..3]
much further down in init_one() past the point where we allocate a (struct
adapter) for PF[0..3]. As a result, we left four of these on ever module remove.

Fix: Allocate adapter structure only for PF4

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |   49 ++++++++++++----------
 1 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 8c34811..e5be511 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -6478,6 +6478,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct port_info *pi;
 	bool highdma = false;
 	struct adapter *adapter = NULL;
+	void __iomem *regs;
 
 	printk_once(KERN_INFO "%s - version %s\n", DRV_DESC, DRV_VERSION);
 
@@ -6494,19 +6495,35 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto out_release_regions;
 	}
 
+	regs = pci_ioremap_bar(pdev, 0);
+	if (!regs) {
+		dev_err(&pdev->dev, "cannot map device registers\n");
+		err = -ENOMEM;
+		goto out_disable_device;
+	}
+
+	/* We control everything through one PF */
+	func = SOURCEPF_GET(readl(regs + PL_WHOAMI));
+	if (func != ent->driver_data) {
+		iounmap(regs);
+		pci_disable_device(pdev);
+		pci_save_state(pdev);        /* to restore SR-IOV later */
+		goto sriov;
+	}
+
 	if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(64))) {
 		highdma = true;
 		err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
 		if (err) {
 			dev_err(&pdev->dev, "unable to obtain 64-bit DMA for "
 				"coherent allocations\n");
-			goto out_disable_device;
+			goto out_unmap_bar0;
 		}
 	} else {
 		err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 		if (err) {
 			dev_err(&pdev->dev, "no usable DMA configuration\n");
-			goto out_disable_device;
+			goto out_unmap_bar0;
 		}
 	}
 
@@ -6518,7 +6535,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
 	if (!adapter) {
 		err = -ENOMEM;
-		goto out_disable_device;
+		goto out_unmap_bar0;
 	}
 
 	adapter->workq = create_singlethread_workqueue("cxgb4");
@@ -6530,20 +6547,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* PCI device has been enabled */
 	adapter->flags |= DEV_ENABLED;
 
-	adapter->regs = pci_ioremap_bar(pdev, 0);
-	if (!adapter->regs) {
-		dev_err(&pdev->dev, "cannot map device registers\n");
-		err = -ENOMEM;
-		goto out_free_adapter;
-	}
-
-	/* We control everything through one PF */
-	func = SOURCEPF_GET(readl(adapter->regs + PL_WHOAMI));
-	if (func != ent->driver_data) {
-		pci_save_state(pdev);        /* to restore SR-IOV later */
-		goto sriov;
-	}
-
+	adapter->regs = regs;
 	adapter->pdev = pdev;
 	adapter->pdev_dev = &pdev->dev;
 	adapter->mbox = func;
@@ -6560,7 +6564,8 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	err = t4_prep_adapter(adapter);
 	if (err)
-		goto out_unmap_bar0;
+		goto out_free_adapter;
+
 
 	if (!is_t4(adapter->params.chip)) {
 		s_qpp = QUEUESPERPAGEPF1 * adapter->fn;
@@ -6577,14 +6582,14 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			dev_err(&pdev->dev,
 				"Incorrect number of egress queues per page\n");
 			err = -EINVAL;
-			goto out_unmap_bar0;
+			goto out_free_adapter;
 		}
 		adapter->bar2 = ioremap_wc(pci_resource_start(pdev, 2),
 		pci_resource_len(pdev, 2));
 		if (!adapter->bar2) {
 			dev_err(&pdev->dev, "cannot map device bar2 region\n");
 			err = -ENOMEM;
-			goto out_unmap_bar0;
+			goto out_free_adapter;
 		}
 	}
 
@@ -6722,13 +6727,13 @@ sriov:
  out_unmap_bar:
 	if (!is_t4(adapter->params.chip))
 		iounmap(adapter->bar2);
- out_unmap_bar0:
-	iounmap(adapter->regs);
  out_free_adapter:
 	if (adapter->workq)
 		destroy_workqueue(adapter->workq);
 
 	kfree(adapter);
+ out_unmap_bar0:
+	iounmap(regs);
  out_disable_device:
 	pci_disable_pcie_error_reporting(pdev);
 	pci_disable_device(pdev);
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH 0/2] DSA Cleanups
From: David Miller @ 2014-09-15 21:24 UTC (permalink / raw)
  To: alexander.h.duyck; +Cc: netdev, f.fainelli, kernel
In-Reply-To: <20140915165543.1261.2520.stgit@ahduyck-bv4.jf.intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Mon, 15 Sep 2014 12:59:14 -0400

> This patch series does two things, first it cleans up the tag_protocol and
> protocol ops being configured seperately.  Second it addresses the desire
> to split DSA away from relying on a MII bus.

This looks ok, series applied, thanks.

^ permalink raw reply

* Re: how to figure out which device a given IFB is connected to
From: Sebastian Moeller @ 2014-09-15 21:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Cong Wang, netdev
In-Reply-To: <20140915135508.5b346e22@urahara>

Hello Mr. Hemminger,


On Sep 15, 2014, at 22:55 , Stephen Hemminger <stephen@networkplumber.org> wrote:

> On Mon, 15 Sep 2014 21:06:31 +0200
> Sebastian Moeller <moeller0@gmx.de> wrote:
> 
>> Hi Mr. Wang,
>> 
>> Thank you for your help.
>> 
>> On Sep 15, 2014, at 18:08 , Cong Wang <cwang@twopensource.com> wrote:
>> 
>>> On Mon, Sep 15, 2014 at 7:37 AM, Sebastian Moeller <moeller0@gmx.de> wrote:
>>>> I am looking for a way of doing the reverse,i.e.  figuring out for a given IFB if it is “connected” to a real interface and, if yes, which interface. Basically, I want to recycle unused IFBs, but want to make sure that they really are unused…
>>>> 
>>> 
>>> There is no way to figure that out.
>> 
>> 	That is rather unfortunate, so my only recourse is to get a list of all interfaces and query each whether it is attached to an fib and prune a list of IFBs so that only the unused ones remain (which is far from elegant ;) ).
> 
> It can be a many to one mapping.
> There are cases where you want multiple incoming devices to all be QoS'd together.

	Good to know, thanks for this information.

Best Regards
	Sebastian Moeller

^ permalink raw reply

* [PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly
From: Bernhard Thaler @ 2014-09-15 21:27 UTC (permalink / raw)
  To: davem, kuznet, jmorris, yoshfuji, kaber, stephen
  Cc: netdev, bridge, linux-kernel, Bernhard Thaler

Ethernet frames are not bridged to correct interface when packets have
been NAT66ed; compared to IPv4 logic in code, IPv6 code does not store
original address (before NAT) on transmit to determine if packet was
NAT66ed on receive to swap addresses back.

Changes added in br_netfilter.c to store original address, compare
against it and determine correct output interface. Changes needed in
netfilter_bridge.h to store IPv6 address in pre-existing union.
Export of ip6_route_input needed to use it in br_netfilter.c.

Problem may only affect systems doing NAT66 and ethernet bridging at
the same time. Tested in NAT66 setup on base of an ethernet bridge.

Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
---
 include/linux/netfilter_bridge.h |    2 +
 net/bridge/br_netfilter.c        |  136 ++++++++++++++++++++++++++++----------
 net/ipv6/route.c                 |    1 +
 3 files changed, 105 insertions(+), 34 deletions(-)

diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
index 8ab1c27..3a9cdcd 100644
--- a/include/linux/netfilter_bridge.h
+++ b/include/linux/netfilter_bridge.h
@@ -2,6 +2,7 @@
 #define __LINUX_BRIDGE_NETFILTER_H
 
 #include <uapi/linux/netfilter_bridge.h>
+#include <uapi/linux/in6.h>
 
 
 enum nf_br_hook_priorities {
@@ -79,6 +80,7 @@ static inline unsigned int nf_bridge_pad(const struct sk_buff *skb)
 struct bridge_skb_cb {
 	union {
 		__be32 ipv4;
+		struct in6_addr ipv6;
 	} daddr;
 };
 
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index a615264..2ae3888 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -35,6 +35,9 @@
 #include <net/ip.h>
 #include <net/ipv6.h>
 #include <net/route.h>
+#include <net/ip6_route.h>
+#include <net/flow.h>
+#include <net/dst.h>
 
 #include <asm/uaccess.h>
 #include "br_private.h"
@@ -42,10 +45,18 @@
 #include <linux/sysctl.h>
 #endif
 
-#define skb_origaddr(skb)	 (((struct bridge_skb_cb *) \
-				 (skb->nf_bridge->data))->daddr.ipv4)
-#define store_orig_dstaddr(skb)	 (skb_origaddr(skb) = ip_hdr(skb)->daddr)
-#define dnat_took_place(skb)	 (skb_origaddr(skb) != ip_hdr(skb)->daddr)
+#define skb_origaddr(skb)		(((struct bridge_skb_cb *)\
+					(skb->nf_bridge->data))->daddr.ipv4)
+#define skb_origaddr_ipv6(skb)		(((struct bridge_skb_cb *)\
+					(skb->nf_bridge->data))->daddr.ipv6)
+#define store_orig_dstaddr(skb)		(skb_origaddr(skb) = ip_hdr(skb)->daddr)
+#define store_orig_dstaddr_ipv6(skb)	(skb_origaddr_ipv6(skb) = \
+					ipv6_hdr(skb)->daddr)
+#define dnat_took_place(skb)		(skb_origaddr(skb) != \
+					ip_hdr(skb)->daddr)
+#define dnat_took_place_ipv6(skb)	(memcmp(&skb_origaddr_ipv6(skb), \
+					&(ipv6_hdr(skb)->daddr), \
+					sizeof(struct in6_addr)) != 0)
 
 #ifdef CONFIG_SYSCTL
 static struct ctl_table_header *brnf_sysctl_header;
@@ -340,36 +351,6 @@ int nf_bridge_copy_header(struct sk_buff *skb)
 	return 0;
 }
 
-/* PF_BRIDGE/PRE_ROUTING *********************************************/
-/* Undo the changes made for ip6tables PREROUTING and continue the
- * bridge PRE_ROUTING hook. */
-static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
-{
-	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
-	struct rtable *rt;
-
-	if (nf_bridge->mask & BRNF_PKT_TYPE) {
-		skb->pkt_type = PACKET_OTHERHOST;
-		nf_bridge->mask ^= BRNF_PKT_TYPE;
-	}
-	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
-
-	rt = bridge_parent_rtable(nf_bridge->physindev);
-	if (!rt) {
-		kfree_skb(skb);
-		return 0;
-	}
-	skb_dst_set_noref(skb, &rt->dst);
-
-	skb->dev = nf_bridge->physindev;
-	nf_bridge_update_protocol(skb);
-	nf_bridge_push_encap_header(skb);
-	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
-		       br_handle_frame_finish, 1);
-
-	return 0;
-}
-
 /* Obtain the correct destination MAC address, while preserving the original
  * source MAC address. If we already know this address, we just copy it. If we
  * don't, we use the neighbour framework to find out. In both cases, we make
@@ -527,6 +508,92 @@ bridged_dnat:
 	return 0;
 }
 
+/* PF_BRIDGE/PRE_ROUTING *********************************************
+ * Undo the changes made for ip6tables PREROUTING and continue the
+ * bridge PRE_ROUTING hook.
+ */
+static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
+{
+	struct net_device *dev = skb->dev;
+	struct ipv6hdr *iph = ipv6_hdr(skb);
+	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
+	struct rtable *rt;
+	struct dst_entry *dst;
+	struct flowi6 fl6 = {
+		.flowi6_iif = skb->dev->ifindex,
+		.daddr = iph->daddr,
+		.saddr = iph->saddr,
+		.flowlabel = ip6_flowinfo(iph),
+		.flowi6_mark = skb->mark,
+		.flowi6_proto = iph->nexthdr,
+	};
+
+	if (nf_bridge->mask & BRNF_PKT_TYPE) {
+		skb->pkt_type = PACKET_OTHERHOST;
+		nf_bridge->mask ^= BRNF_PKT_TYPE;
+	}
+	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
+
+	if (dnat_took_place_ipv6(skb)) {
+		ip6_route_input(skb);
+		/* ip6_route_input is void function,
+		 * no int returned as in ip4_route_input
+		 * changes value of skb->_skb_refdst) on success
+		 */
+		if (skb->_skb_refdst == 0) {
+			struct in_device *in_dev = __in_dev_get_rcu(dev);
+
+			if (!in_dev || IN_DEV_FORWARD(in_dev))
+				goto free_skb;
+
+			dst = ip6_route_output(dev_net(dev), skb->sk, &fl6);
+			if (!IS_ERR(dst)) {
+				/* - Bridged-and-DNAT'ed traffic doesn't
+				 *   require ip_forwarding.
+				 */
+				if (dst->dev == dev) {
+					skb_dst_set(skb, dst);
+					goto bridged_dnat;
+				}
+				dst_release(dst);
+			}
+free_skb:
+			kfree_skb(skb);
+			return 0;
+		} else {
+			if (skb_dst(skb)->dev == dev) {
+bridged_dnat:
+				skb->dev = nf_bridge->physindev;
+				nf_bridge_update_protocol(skb);
+				nf_bridge_push_encap_header(skb);
+				NF_HOOK_THRESH(NFPROTO_BRIDGE,
+					       NF_BR_PRE_ROUTING,
+					       skb, skb->dev, NULL,
+					       br_nf_pre_routing_finish_bridge,
+					       1);
+				return 0;
+			}
+			memcpy(eth_hdr(skb)->h_dest, dev->dev_addr, ETH_ALEN);
+			skb->pkt_type = PACKET_HOST;
+		}
+	} else {
+		rt = bridge_parent_rtable(nf_bridge->physindev);
+		if (!rt) {
+			kfree_skb(skb);
+			return 0;
+		}
+		skb_dst_set_noref(skb, &rt->dst);
+	}
+
+	skb->dev = nf_bridge->physindev;
+	nf_bridge_update_protocol(skb);
+	nf_bridge_push_encap_header(skb);
+	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
+		       br_handle_frame_finish, 1);
+
+	return 0;
+}
+
 static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
 {
 	struct net_device *vlan, *br;
@@ -658,6 +725,7 @@ static unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
 	if (!setup_pre_routing(skb))
 		return NF_DROP;
 
+	store_orig_dstaddr_ipv6(skb);
 	skb->protocol = htons(ETH_P_IPV6);
 	NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, skb->dev, NULL,
 		br_nf_pre_routing_finish_ipv6);
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f23fbd2..e328905 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1017,6 +1017,7 @@ void ip6_route_input(struct sk_buff *skb)
 
 	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
 }
+EXPORT_SYMBOL(ip6_route_input);
 
 static struct rt6_info *ip6_pol_route_output(struct net *net, struct fib6_table *table,
 					     struct flowi6 *fl6, int flags)
-- 
1.7.10.4

^ permalink raw reply related

* Re: [Patch net-next 3/5] net_sched: fix suspicious RCU usage in tcindex_classify()
From: John Fastabend @ 2014-09-15 21:29 UTC (permalink / raw)
  To: Cong Wang, netdev; +Cc: john.fastabend, David S. Miller
In-Reply-To: <1410815210-6693-4-git-send-email-xiyou.wangcong@gmail.com>

On 09/15/2014 02:06 PM, Cong Wang wrote:
> This patch fixes the following kernel warning:
> 
> [   44.805900] [ INFO: suspicious RCU usage. ]
> [   44.808946] 3.17.0-rc4+ #610 Not tainted
> [   44.811831] -------------------------------
> [   44.814873] net/sched/cls_tcindex.c:84 suspicious rcu_dereference_check() usage!
> 
> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  net/sched/cls_tcindex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
> index 16ec1ed..f276de5 100644
> --- a/net/sched/cls_tcindex.c
> +++ b/net/sched/cls_tcindex.c
> @@ -81,7 +81,7 @@ tcindex_lookup(struct tcindex_data *p, u16 key)
>  static int tcindex_classify(struct sk_buff *skb, const struct tcf_proto *tp,
>  			    struct tcf_result *res)
>  {
> -	struct tcindex_data *p = rcu_dereference(tp->root);
> +	struct tcindex_data *p = rcu_dereference_bh(tp->root);
>  	struct tcindex_filter_result *f;
>  	int key = (skb->tc_index & p->mask) >> p->shift;
>  
> 

Thanks for catching/fixing. I also missed cls_bpf.c case I'll send a fix
in a moment.


Acked-by: John Fastabend <john.r.fastabend@intel.com>

^ permalink raw reply

* Re: [Patch net-next] net_sched: fix suspicious RCU usage in cls_bpf_classify()
From: John Fastabend @ 2014-09-15 21:30 UTC (permalink / raw)
  To: Cong Wang, netdev; +Cc: john.fastabend, David S. Miller
In-Reply-To: <1410816110-10354-1-git-send-email-xiyou.wangcong@gmail.com>

On 09/15/2014 02:21 PM, Cong Wang wrote:
> Fixes: commit 1f947bf151e90ec0baad2948 ("net: sched: rcu'ify cls_bpf")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  net/sched/cls_bpf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> index 6a7386e..4e3f5bf 100644
> --- a/net/sched/cls_bpf.c
> +++ b/net/sched/cls_bpf.c
> @@ -52,7 +52,7 @@ static const struct nla_policy bpf_policy[TCA_BPF_MAX + 1] = {
>  static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
>  			    struct tcf_result *res)
>  {
> -	struct cls_bpf_head *head = rcu_dereference(tp->root);
> +	struct cls_bpf_head *head = rcu_dereference_bh(tp->root);
>  	struct cls_bpf_prog *prog;
>  	int ret;
>  
> 

you beat me to it.

Acked-by: John Fastabend <john.r.fastabend@intel.com>

^ permalink raw reply

* Re: [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT
From: David Miller @ 2014-09-15 21:32 UTC (permalink / raw)
  To: Frank.Li; +Cc: b38611, netdev, lznuaa, shawn.guo, linux-arm-kernel
In-Reply-To: <1410801177-15872-2-git-send-email-Frank.Li@freescale.com>

From: <Frank.Li@freescale.com>
Date: Tue, 16 Sep 2014 01:12:54 +0800

> -		dev_err(&pdev->dev, "Invalidate num_tx(=%d), fail back to 1\n",
> +		dev_warn(&pdev->dev, "Invalid num_tx(=%d), fall back to 1\n",
>  			*num_tx);

Any time you change the function name of a multi-line function call, you must
be mindful to adjust the indention of the subsequent argument lines, if
necessary.

>  	if (*num_rx < 1 || *num_rx > FEC_ENET_MAX_RX_QS) {
> -		dev_err(&pdev->dev, "Invalidate num_rx(=%d), fail back to 1\n",
> +		dev_warn(&pdev->dev, "Invalid num_rx(=%d), fall back to 1\n",
>  			*num_rx);

Likewise.

^ permalink raw reply

* Re: [Patch net-next 2/4] net: fec: add interrupt coalescence feature support
From: David Miller @ 2014-09-15 21:34 UTC (permalink / raw)
  To: Frank.Li; +Cc: b38611, netdev, lznuaa, shawn.guo, linux-arm-kernel
In-Reply-To: <1410801177-15872-3-git-send-email-Frank.Li@freescale.com>

From: <Frank.Li@freescale.com>
Date: Tue, 16 Sep 2014 01:12:55 +0800

>  	unsigned int tx_align;
>  	unsigned int rx_align;
> +
> +	/* hw interrupt coalesce */
> +	uint rx_pkts_itr;
> +	uint rx_time_itr;
> +	uint tx_pkts_itr;
> +	uint tx_time_itr;

Please use explicit "unsigned int" just like the lines right above the ones
you are adding.

> +	/* Must be greater than zero to avoid unpredictable behavior */
> +	if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
> +		!fep->tx_time_itr || !fep->tx_pkts_itr)

This is not indented properly.

On the second and subsequent lines of a multi-line conditional, things
must start exactly at the first column after the openning parenthesis
of the first line.

^ permalink raw reply

* Re: [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues
From: David Miller @ 2014-09-15 21:35 UTC (permalink / raw)
  To: Frank.Li; +Cc: b38611, netdev, lznuaa, shawn.guo, linux-arm-kernel
In-Reply-To: <1410801177-15872-5-git-send-email-Frank.Li@freescale.com>

From: <Frank.Li@freescale.com>
Date: Tue, 16 Sep 2014 01:12:57 +0800

> @@ -111,6 +111,13 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
>   *   independent rings
>   */
>  #define FEC_QUIRK_HAS_AVB		(1 << 8)
> +/*
> + * There is a TDAR race condition for mutliQ when the software sets TDAR
> + * and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
> + * This will cause the udma_tx and udma_tx_arbiter state machines to hang.
> + * The issue exist at i.MX6SX enet IP.
> + */
> +#define FEC_QUIRK_TKT210582		(1 << 9)

Networking comments should be of the form:

	/* Like
	 * this.
	 */

>  	/* Trigger transmission start */
> -	writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
> +	if (!(id_entry->driver_data & FEC_QUIRK_TKT210582) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)))
> +		writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));

This conditional is not indented properly, see my feedback for patch #2.

^ permalink raw reply

* Re: [PATCH] bridge: Fix br_should_learn to check vlan_enabled
From: David Miller @ 2014-09-15 21:38 UTC (permalink / raw)
  To: vyasevich; +Cc: netdev, toshiaki.makita1, vyasevic
In-Reply-To: <1410809066-4434-1-git-send-email-vyasevic@redhat.com>

From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Mon, 15 Sep 2014 15:24:26 -0400

> As Toshiaki Makita pointed out, the BRIDGE_INPUT_SKB_CB will
> not be initialized in br_should_learn() as that function
> is called only from br_handle_local_finish().  That is
> an input handler for link-local ethernet traffic so it perfectly
> correct to check br->vlan_enabled here.
> 
> Reported-by: Toshiaki Makita<toshiaki.makita1@gmail.com>
> Fixes: 20adfa1 bridge: Check if vlan filtering is enabled only once.
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>

Applied, thanks Vlad.

^ permalink raw reply

* Re: [PATCH 1/1] bridge: Fix NAT66ed IPv6 packets not being bridged correctly
From: David Miller @ 2014-09-15 21:40 UTC (permalink / raw)
  To: bernhard.thaler
  Cc: yoshfuji, netdev, bridge, jmorris, linux-kernel, stephen,
	netfilter-devel, kuznet, kaber, pablo
In-Reply-To: <1410816448-4401-1-git-send-email-bernhard.thaler@wvnet.at>

From: Bernhard Thaler <bernhard.thaler@wvnet.at>
Date: Mon, 15 Sep 2014 23:27:28 +0200

CC:'ing netfilter-devel and the netfilter maintainer, which is
probably the primary place this patch should have been submitted.

> Ethernet frames are not bridged to correct interface when packets have
> been NAT66ed; compared to IPv4 logic in code, IPv6 code does not store
> original address (before NAT) on transmit to determine if packet was
> NAT66ed on receive to swap addresses back.
> 
> Changes added in br_netfilter.c to store original address, compare
> against it and determine correct output interface. Changes needed in
> netfilter_bridge.h to store IPv6 address in pre-existing union.
> Export of ip6_route_input needed to use it in br_netfilter.c.
> 
> Problem may only affect systems doing NAT66 and ethernet bridging at
> the same time. Tested in NAT66 setup on base of an ethernet bridge.
> 
> Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
> ---
>  include/linux/netfilter_bridge.h |    2 +
>  net/bridge/br_netfilter.c        |  136 ++++++++++++++++++++++++++++----------
>  net/ipv6/route.c                 |    1 +
>  3 files changed, 105 insertions(+), 34 deletions(-)
> 
> diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
> index 8ab1c27..3a9cdcd 100644
> --- a/include/linux/netfilter_bridge.h
> +++ b/include/linux/netfilter_bridge.h
> @@ -2,6 +2,7 @@
>  #define __LINUX_BRIDGE_NETFILTER_H
>  
>  #include <uapi/linux/netfilter_bridge.h>
> +#include <uapi/linux/in6.h>
>  
>  
>  enum nf_br_hook_priorities {
> @@ -79,6 +80,7 @@ static inline unsigned int nf_bridge_pad(const struct sk_buff *skb)
>  struct bridge_skb_cb {
>  	union {
>  		__be32 ipv4;
> +		struct in6_addr ipv6;
>  	} daddr;
>  };
>  
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index a615264..2ae3888 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -35,6 +35,9 @@
>  #include <net/ip.h>
>  #include <net/ipv6.h>
>  #include <net/route.h>
> +#include <net/ip6_route.h>
> +#include <net/flow.h>
> +#include <net/dst.h>
>  
>  #include <asm/uaccess.h>
>  #include "br_private.h"
> @@ -42,10 +45,18 @@
>  #include <linux/sysctl.h>
>  #endif
>  
> -#define skb_origaddr(skb)	 (((struct bridge_skb_cb *) \
> -				 (skb->nf_bridge->data))->daddr.ipv4)
> -#define store_orig_dstaddr(skb)	 (skb_origaddr(skb) = ip_hdr(skb)->daddr)
> -#define dnat_took_place(skb)	 (skb_origaddr(skb) != ip_hdr(skb)->daddr)
> +#define skb_origaddr(skb)		(((struct bridge_skb_cb *)\
> +					(skb->nf_bridge->data))->daddr.ipv4)
> +#define skb_origaddr_ipv6(skb)		(((struct bridge_skb_cb *)\
> +					(skb->nf_bridge->data))->daddr.ipv6)
> +#define store_orig_dstaddr(skb)		(skb_origaddr(skb) = ip_hdr(skb)->daddr)
> +#define store_orig_dstaddr_ipv6(skb)	(skb_origaddr_ipv6(skb) = \
> +					ipv6_hdr(skb)->daddr)
> +#define dnat_took_place(skb)		(skb_origaddr(skb) != \
> +					ip_hdr(skb)->daddr)
> +#define dnat_took_place_ipv6(skb)	(memcmp(&skb_origaddr_ipv6(skb), \
> +					&(ipv6_hdr(skb)->daddr), \
> +					sizeof(struct in6_addr)) != 0)
>  
>  #ifdef CONFIG_SYSCTL
>  static struct ctl_table_header *brnf_sysctl_header;
> @@ -340,36 +351,6 @@ int nf_bridge_copy_header(struct sk_buff *skb)
>  	return 0;
>  }
>  
> -/* PF_BRIDGE/PRE_ROUTING *********************************************/
> -/* Undo the changes made for ip6tables PREROUTING and continue the
> - * bridge PRE_ROUTING hook. */
> -static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
> -{
> -	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> -	struct rtable *rt;
> -
> -	if (nf_bridge->mask & BRNF_PKT_TYPE) {
> -		skb->pkt_type = PACKET_OTHERHOST;
> -		nf_bridge->mask ^= BRNF_PKT_TYPE;
> -	}
> -	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
> -
> -	rt = bridge_parent_rtable(nf_bridge->physindev);
> -	if (!rt) {
> -		kfree_skb(skb);
> -		return 0;
> -	}
> -	skb_dst_set_noref(skb, &rt->dst);
> -
> -	skb->dev = nf_bridge->physindev;
> -	nf_bridge_update_protocol(skb);
> -	nf_bridge_push_encap_header(skb);
> -	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
> -		       br_handle_frame_finish, 1);
> -
> -	return 0;
> -}
> -
>  /* Obtain the correct destination MAC address, while preserving the original
>   * source MAC address. If we already know this address, we just copy it. If we
>   * don't, we use the neighbour framework to find out. In both cases, we make
> @@ -527,6 +508,92 @@ bridged_dnat:
>  	return 0;
>  }
>  
> +/* PF_BRIDGE/PRE_ROUTING *********************************************
> + * Undo the changes made for ip6tables PREROUTING and continue the
> + * bridge PRE_ROUTING hook.
> + */
> +static int br_nf_pre_routing_finish_ipv6(struct sk_buff *skb)
> +{
> +	struct net_device *dev = skb->dev;
> +	struct ipv6hdr *iph = ipv6_hdr(skb);
> +	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> +	struct rtable *rt;
> +	struct dst_entry *dst;
> +	struct flowi6 fl6 = {
> +		.flowi6_iif = skb->dev->ifindex,
> +		.daddr = iph->daddr,
> +		.saddr = iph->saddr,
> +		.flowlabel = ip6_flowinfo(iph),
> +		.flowi6_mark = skb->mark,
> +		.flowi6_proto = iph->nexthdr,
> +	};
> +
> +	if (nf_bridge->mask & BRNF_PKT_TYPE) {
> +		skb->pkt_type = PACKET_OTHERHOST;
> +		nf_bridge->mask ^= BRNF_PKT_TYPE;
> +	}
> +	nf_bridge->mask ^= BRNF_NF_BRIDGE_PREROUTING;
> +
> +	if (dnat_took_place_ipv6(skb)) {
> +		ip6_route_input(skb);
> +		/* ip6_route_input is void function,
> +		 * no int returned as in ip4_route_input
> +		 * changes value of skb->_skb_refdst) on success
> +		 */
> +		if (skb->_skb_refdst == 0) {
> +			struct in_device *in_dev = __in_dev_get_rcu(dev);
> +
> +			if (!in_dev || IN_DEV_FORWARD(in_dev))
> +				goto free_skb;
> +
> +			dst = ip6_route_output(dev_net(dev), skb->sk, &fl6);
> +			if (!IS_ERR(dst)) {
> +				/* - Bridged-and-DNAT'ed traffic doesn't
> +				 *   require ip_forwarding.
> +				 */
> +				if (dst->dev == dev) {
> +					skb_dst_set(skb, dst);
> +					goto bridged_dnat;
> +				}
> +				dst_release(dst);
> +			}
> +free_skb:
> +			kfree_skb(skb);
> +			return 0;
> +		} else {
> +			if (skb_dst(skb)->dev == dev) {
> +bridged_dnat:
> +				skb->dev = nf_bridge->physindev;
> +				nf_bridge_update_protocol(skb);
> +				nf_bridge_push_encap_header(skb);
> +				NF_HOOK_THRESH(NFPROTO_BRIDGE,
> +					       NF_BR_PRE_ROUTING,
> +					       skb, skb->dev, NULL,
> +					       br_nf_pre_routing_finish_bridge,
> +					       1);
> +				return 0;
> +			}
> +			memcpy(eth_hdr(skb)->h_dest, dev->dev_addr, ETH_ALEN);
> +			skb->pkt_type = PACKET_HOST;
> +		}
> +	} else {
> +		rt = bridge_parent_rtable(nf_bridge->physindev);
> +		if (!rt) {
> +			kfree_skb(skb);
> +			return 0;
> +		}
> +		skb_dst_set_noref(skb, &rt->dst);
> +	}
> +
> +	skb->dev = nf_bridge->physindev;
> +	nf_bridge_update_protocol(skb);
> +	nf_bridge_push_encap_header(skb);
> +	NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_PRE_ROUTING, skb, skb->dev, NULL,
> +		       br_handle_frame_finish, 1);
> +
> +	return 0;
> +}
> +
>  static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
>  {
>  	struct net_device *vlan, *br;
> @@ -658,6 +725,7 @@ static unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
>  	if (!setup_pre_routing(skb))
>  		return NF_DROP;
>  
> +	store_orig_dstaddr_ipv6(skb);
>  	skb->protocol = htons(ETH_P_IPV6);
>  	NF_HOOK(NFPROTO_IPV6, NF_INET_PRE_ROUTING, skb, skb->dev, NULL,
>  		br_nf_pre_routing_finish_ipv6);
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f23fbd2..e328905 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1017,6 +1017,7 @@ void ip6_route_input(struct sk_buff *skb)
>  
>  	skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags));
>  }
> +EXPORT_SYMBOL(ip6_route_input);
>  
>  static struct rt6_info *ip6_pol_route_output(struct net *net, struct fib6_table *table,
>  					     struct flowi6 *fl6, int flags)
> -- 
> 1.7.10.4
> 

^ permalink raw reply

* Re: [Patch net-next 2/5] net_sched: fix memory leak in cls_tcindex
From: John Fastabend @ 2014-09-15 21:41 UTC (permalink / raw)
  To: Cong Wang, netdev; +Cc: john.fastabend, David S. Miller
In-Reply-To: <1410815210-6693-3-git-send-email-xiyou.wangcong@gmail.com>

On 09/15/2014 02:06 PM, Cong Wang wrote:
> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  net/sched/cls_tcindex.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
> index a02ca72..16ec1ed 100644
> --- a/net/sched/cls_tcindex.c
> +++ b/net/sched/cls_tcindex.c
> @@ -243,7 +243,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>  	 */
>  	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
>  	if (!cp)
> -		return -ENOMEM;
> +		goto errout;

but you need to set 'err = -ENOMEM' then.

>  
>  	cp->mask = p->mask;
>  	cp->shift = p->shift;
> @@ -257,6 +257,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>  				      sizeof(*r) * cp->hash, GFP_KERNEL);
>  		if (!cp->perfect)
>  			goto errout;
> +		balloc = 1;

Actually can we just get rid of the balloc here altogether and
remove the checks in errout_alloc so that cp->perfect and cp->h
are freed unconditionally? They should be NULL if they are not
being used because of the kzalloc.

>  	}
>  	cp->h = p->h;
>  
> @@ -285,9 +286,9 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>  	if (cp->perfect) {
>  		if (!valid_perfect_hash(cp) ||
>  		    cp->hash > cp->alloc_hash)
> -			goto errout;
> +			goto errout_alloc;
>  	} else if (cp->h && cp->hash != cp->alloc_hash) {
> -		goto errout;
> +		goto errout_alloc;
>  	}
>  
>  	err = -EINVAL;
> @@ -314,7 +315,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>  	 */
>  	if (cp->perfect || valid_perfect_hash(cp))
>  		if (handle >= cp->alloc_hash)
> -			goto errout;
> +			goto errout_alloc;
>  
>  
>  	err = -ENOMEM;
> @@ -324,7 +325,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>  
>  			cp->perfect = kcalloc(cp->hash, sizeof(*r), GFP_KERNEL);
>  			if (!cp->perfect)
> -				goto errout;
> +				goto errout_alloc;
>  			for (i = 0; i < cp->hash; i++)
>  				tcf_exts_init(&cp->perfect[i].exts,
>  					      TCA_TCINDEX_ACT,
> @@ -338,7 +339,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>  				       GFP_KERNEL);
>  
>  			if (!hash)
> -				goto errout;
> +				goto errout_alloc;
>  
>  			cp->h = hash;
>  			balloc = 2;
> 

^ permalink raw reply

* Re: [PATCH net] cxgb4: Don't allocate adapter structure for all PF's
From: David Miller @ 2014-09-15 21:43 UTC (permalink / raw)
  To: hariprasad; +Cc: netdev, leedom, nirranjan, kumaras, anish
In-Reply-To: <1410816526-19355-1-git-send-email-hariprasad@chelsio.com>

From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Tue, 16 Sep 2014 02:58:46 +0530

> commit 35b1de557970 ("rdma/cxgb4: Fixes cxgb4 probe failure in VM when PF is
> exposed through PCI Passthrough") moved the code to check for SR-IOV PF[0..3]
> much further down in init_one() past the point where we allocate a (struct
> adapter) for PF[0..3]. As a result, we left four of these on ever module remove.
> 
> Fix: Allocate adapter structure only for PF4
> 
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>

Applied.

^ permalink raw reply

* Re: [Patch net-next] net_sched: fix suspicious RCU usage in cls_bpf_classify()
From: David Miller @ 2014-09-15 21:43 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, john.fastabend
In-Reply-To: <1410816110-10354-1-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 15 Sep 2014 14:21:50 -0700

> Fixes: commit 1f947bf151e90ec0baad2948 ("net: sched: rcu'ify cls_bpf")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net 0/2] Fix randconfig errros in bnx2i/bnx2fc caused by IPV6
From: David Miller @ 2014-09-15 21:45 UTC (permalink / raw)
  To: anish; +Cc: netdev, linux-scsi, JBottomley, mchan, eddie.wai, rdunlap,
	jim.epost
In-Reply-To: <1410813921-24400-1-git-send-email-anish@chelsio.com>

From: Anish Bhatt <anish@chelsio.com>
Date: Mon, 15 Sep 2014 13:45:19 -0700

> Just like CNIC bnx2i/bnx2fc also have their tristate dependent on IPV6, however
> using the same solution as CNIC can cause recursive dependecies during make.
> 
> Based on suggestions by Randy Dunlap, SCSI_NETLINK now depends on NET instead
>  of selecting NET. Second patch fixes the actual randconfig error.
> 
> Entire thread can be followed here : https://lkml.org/lkml/2014/9/9/500
> -Anish
> 
> Fixes: c99d667e8527 ("cnic : Cleanup CONFIG_IPV6 & VLAN check")

Series applied, thank you.

^ permalink raw reply

* Re: [PATCH] net: can: usb: peak_usb: pcan_usb_core.c: Cleaning up missing null-terminate in conjunction with strncpy
From: Rickard Strandqvist @ 2014-09-15 21:47 UTC (permalink / raw)
  To: David Laight
  Cc: Marc Kleine-Budde, Wolfgang Grandegger, Oliver Hartkopp,
	Stephane Grosjean, Alexey Khoroshilov, Christopher R. Baker,
	linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17491F83@AcuExch.aculab.com>

2014-09-15 10:47 GMT+02:00 David Laight <David.Laight@aculab.com>:
> From: Marc Kleine-Budde [
>> On 09/15/2014 10:28 AM, David Laight wrote:
>> > From: Rickard Strandqvist
>> > ...
>> >> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
>> > ...
>> >> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
>> >> b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
>> >> index 644e6ab..d4fe8ac 100644
>> >> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
>> >> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
>> >> @@ -830,7 +830,7 @@ static void peak_usb_disconnect(struct usb_interface *intf)
>> >>            char name[IFNAMSIZ];
>> >>
>> >>            dev->state &= ~PCAN_USB_STATE_CONNECTED;
>> >> -          strncpy(name, netdev->name, IFNAMSIZ);
>> >> +          strlcpy(name, netdev->name, IFNAMSIZ);
>> >>
>> >>            unregister_netdev(netdev);
>> >>            free_candev(netdev);
>> >
>> > Or:
>> >     char name[sizeof netdev->name];
>> >     memcpy(name, netdev->name, sizeof netdev->name);
>>
>> I would be "sizeof(foo)" in kernel coding style,
> But not in mine :-)
> sizeof is an operator, not a function, it's argument can be (type).
>
>> but let's have a look at the original code:
>>
>>                 struct net_device *netdev = dev->netdev;
>>                 char name[IFNAMSIZ];
>>
>>                 dev->state &= ~PCAN_USB_STATE_CONNECTED;
>>                 strncpy(name, netdev->name, IFNAMSIZ);
>>
>>                 unregister_netdev(netdev);
>>                 free_candev(netdev);
>>
>>                 kfree(dev->cmd_buf);
>>                 dev->next_siblings = NULL;
>>                 if (dev->adapter->dev_free)
>>                         dev->adapter->dev_free(dev);
>>
>>                 dev_info(&intf->dev, "%s removed\n", name);
>>
>> I think it's save to use:
>>
>>     dev_info(&intf->dev, "%s removed\n", netdev_name(dev->netdev));
>>
>> instead of doing the str?cpy() in the first place. But why not use:
>>
>>     netdev_info(dev->netdev, "removed\n");
>>
>> Is the USB device information lost when using netdev_info()?
>
> My guess is it avoids a 'use after free' - but I'm not going to
> dig that far.
>
> Another issue with blindly replacing strncpy() with strlcpy()
> (which doesn't affect the above) is when copying status to userspace.
>
>         David


Hi all

I have been more and more careful so I did not introduce the type of
error that can arise by blindly replacing strncpy to strlcpy.
But this is one of the most apparent where there will not be a problem.

I liked the variant:
char name[sizeof(netdev->name)];

But dislike and do not understand what the point would be with memcpy variant.


Kind regards
Rickard Strandqvist

^ permalink raw reply

* [PATCH net-next] net: dsa: fix mii_bus to host_dev replacement
From: Florian Fainelli @ 2014-09-15 21:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, alexander.h.duyck, Florian Fainelli

dsa_of_probe() still used cd->mii_bus instead of cd->host_dev when
building with CONFIG_OF=y. Fix this by making the replacement here as
well.

Fixes: b4d2394d01b ("dsa: Replace mii_bus with a generic host device")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index b34d6978d773..6e40928ec0e7 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -440,7 +440,7 @@ static int dsa_of_probe(struct platform_device *pdev)
 		cd = &pd->chip[chip_index];
 
 		cd->of_node = child;
-		cd->mii_bus = &mdio_bus->dev;
+		cd->host_dev = &mdio_bus->dev;
 
 		sw_addr = of_get_property(child, "reg", NULL);
 		if (!sw_addr)
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 2/2] dsa: Replace mii_bus with a generic host device
From: Florian Fainelli @ 2014-09-15 21:48 UTC (permalink / raw)
  To: Alexander Duyck, netdev; +Cc: kernel, davem
In-Reply-To: <20140915170024.1261.97226.stgit@ahduyck-bv4.jf.intel.com>

On 09/15/2014 10:00 AM, Alexander Duyck wrote:
> This change makes it so that instead of passing and storing a mii_bus we
> instead pass and store a host_dev.  From there we can test to determine the
> exact type of device, and can verify it is the correct device for our switch.
> 
> So for example it would be possible to pass a device pointer from a pci_dev
> and instead of checking for a PHY ID we could check for a vendor and/or device
> ID.

This introduce a small build failure with CONFIG_OF for which I will
send a patch shortly. Other than that, this is definitively a step in
the right direction, thanks!

> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>  arch/arm/plat-orion/common.c      |    2 +-
>  drivers/net/dsa/bcm_sf2.c         |    2 +-
>  drivers/net/dsa/mv88e6060.c       |   13 +++++++++----
>  drivers/net/dsa/mv88e6123_61_65.c |    6 +++++-
>  drivers/net/dsa/mv88e6131.c       |    6 +++++-
>  drivers/net/dsa/mv88e6171.c       |    6 +++++-
>  drivers/net/dsa/mv88e6xxx.c       |    4 ++--
>  include/net/dsa.h                 |    9 +++++----
>  net/dsa/dsa.c                     |   24 ++++++++----------------
>  net/dsa/slave.c                   |    2 +-
>  10 files changed, 42 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> index 3ec6e8e..f5b00f4 100644
> --- a/arch/arm/plat-orion/common.c
> +++ b/arch/arm/plat-orion/common.c
> @@ -499,7 +499,7 @@ void __init orion_ge00_switch_init(struct dsa_platform_data *d, int irq)
>  
>  	d->netdev = &orion_ge00.dev;
>  	for (i = 0; i < d->nr_chips; i++)
> -		d->chip[i].mii_bus = &orion_ge00_shared.dev;
> +		d->chip[i].host_dev = &orion_ge00_shared.dev;
>  	orion_switch_device.dev.platform_data = d;
>  
>  	platform_device_register(&orion_switch_device);
> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
> index e9918c7..02d7db3 100644
> --- a/drivers/net/dsa/bcm_sf2.c
> +++ b/drivers/net/dsa/bcm_sf2.c
> @@ -129,7 +129,7 @@ static int bcm_sf2_sw_get_sset_count(struct dsa_switch *ds)
>  	return BCM_SF2_STATS_SIZE;
>  }
>  
> -static char *bcm_sf2_sw_probe(struct mii_bus *bus, int sw_addr)
> +static char *bcm_sf2_sw_probe(struct device *host_dev, int sw_addr)
>  {
>  	return "Broadcom Starfighter 2";
>  }
> diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
> index d8037c1..776e965 100644
> --- a/drivers/net/dsa/mv88e6060.c
> +++ b/drivers/net/dsa/mv88e6060.c
> @@ -21,7 +21,8 @@
>  
>  static int reg_read(struct dsa_switch *ds, int addr, int reg)
>  {
> -	return mdiobus_read(ds->master_mii_bus, ds->pd->sw_addr + addr, reg);
> +	return mdiobus_read(to_mii_bus(ds->master_dev),
> +			    ds->pd->sw_addr + addr, reg);
>  }
>  
>  #define REG_READ(addr, reg)					\
> @@ -37,8 +38,8 @@ static int reg_read(struct dsa_switch *ds, int addr, int reg)
>  
>  static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
>  {
> -	return mdiobus_write(ds->master_mii_bus, ds->pd->sw_addr + addr,
> -			     reg, val);
> +	return mdiobus_write(to_mii_bus(ds->master_dev),
> +			     ds->pd->sw_addr + addr, reg, val);
>  }
>  
>  #define REG_WRITE(addr, reg, val)				\
> @@ -50,10 +51,14 @@ static int reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
>  			return __ret;				\
>  	})
>  
> -static char *mv88e6060_probe(struct mii_bus *bus, int sw_addr)
> +static char *mv88e6060_probe(struct device *host_dev, int sw_addr)
>  {
> +	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
>  	int ret;
>  
> +	if (bus == NULL)
> +		return NULL;
> +
>  	ret = mdiobus_read(bus, sw_addr + REG_PORT(0), 0x03);
>  	if (ret >= 0) {
>  		ret &= 0xfff0;
> diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
> index 975774f..a332c53 100644
> --- a/drivers/net/dsa/mv88e6123_61_65.c
> +++ b/drivers/net/dsa/mv88e6123_61_65.c
> @@ -17,10 +17,14 @@
>  #include <net/dsa.h>
>  #include "mv88e6xxx.h"
>  
> -static char *mv88e6123_61_65_probe(struct mii_bus *bus, int sw_addr)
> +static char *mv88e6123_61_65_probe(struct device *host_dev, int sw_addr)
>  {
> +	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
>  	int ret;
>  
> +	if (bus == NULL)
> +		return NULL;
> +
>  	ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), 0x03);
>  	if (ret >= 0) {
>  		if (ret == 0x1212)
> diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
> index 35541f2..244c735 100644
> --- a/drivers/net/dsa/mv88e6131.c
> +++ b/drivers/net/dsa/mv88e6131.c
> @@ -22,10 +22,14 @@
>  #define ID_6095		0x0950
>  #define ID_6131		0x1060
>  
> -static char *mv88e6131_probe(struct mii_bus *bus, int sw_addr)
> +static char *mv88e6131_probe(struct device *host_dev, int sw_addr)
>  {
> +	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
>  	int ret;
>  
> +	if (bus == NULL)
> +		return NULL;
> +
>  	ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), 0x03);
>  	if (ret >= 0) {
>  		ret &= 0xfff0;
> diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
> index 03a7006..6365e30 100644
> --- a/drivers/net/dsa/mv88e6171.c
> +++ b/drivers/net/dsa/mv88e6171.c
> @@ -17,10 +17,14 @@
>  #include <net/dsa.h>
>  #include "mv88e6xxx.h"
>  
> -static char *mv88e6171_probe(struct mii_bus *bus, int sw_addr)
> +static char *mv88e6171_probe(struct device *host_dev, int sw_addr)
>  {
> +	struct mii_bus *bus = dsa_host_dev_to_mii_bus(host_dev);
>  	int ret;
>  
> +	if (bus == NULL)
> +		return NULL;
> +
>  	ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), 0x03);
>  	if (ret >= 0) {
>  		if ((ret & 0xfff0) == 0x1710)
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 901d2a9..d6f6428 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -78,7 +78,7 @@ int mv88e6xxx_reg_read(struct dsa_switch *ds, int addr, int reg)
>  	int ret;
>  
>  	mutex_lock(&ps->smi_mutex);
> -	ret = __mv88e6xxx_reg_read(ds->master_mii_bus,
> +	ret = __mv88e6xxx_reg_read(to_mii_bus(ds->master_dev),
>  				   ds->pd->sw_addr, addr, reg);
>  	mutex_unlock(&ps->smi_mutex);
>  
> @@ -122,7 +122,7 @@ int mv88e6xxx_reg_write(struct dsa_switch *ds, int addr, int reg, u16 val)
>  	int ret;
>  
>  	mutex_lock(&ps->smi_mutex);
> -	ret = __mv88e6xxx_reg_write(ds->master_mii_bus,
> +	ret = __mv88e6xxx_reg_write(to_mii_bus(ds->master_dev),
>  				    ds->pd->sw_addr, addr, reg, val);
>  	mutex_unlock(&ps->smi_mutex);
>  
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index a55c4e6..c779e9b 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -34,7 +34,7 @@ struct dsa_chip_data {
>  	/*
>  	 * How to access the switch configuration registers.
>  	 */
> -	struct device	*mii_bus;
> +	struct device	*host_dev;
>  	int		sw_addr;
>  
>  	/* Device tree node pointer for this specific switch chip
> @@ -134,9 +134,9 @@ struct dsa_switch {
>  	struct dsa_switch_driver	*drv;
>  
>  	/*
> -	 * Reference to mii bus to use.
> +	 * Reference to host device to use.
>  	 */
> -	struct mii_bus		*master_mii_bus;
> +	struct device		*master_dev;
>  
>  	/*
>  	 * Slave mii_bus and devices for the individual ports.
> @@ -178,7 +178,7 @@ struct dsa_switch_driver {
>  	/*
>  	 * Probing and setup.
>  	 */
> -	char	*(*probe)(struct mii_bus *bus, int sw_addr);
> +	char	*(*probe)(struct device *host_dev, int sw_addr);
>  	int	(*setup)(struct dsa_switch *ds);
>  	int	(*set_addr)(struct dsa_switch *ds, u8 *addr);
>  
> @@ -213,6 +213,7 @@ struct dsa_switch_driver {
>  
>  void register_switch_driver(struct dsa_switch_driver *type);
>  void unregister_switch_driver(struct dsa_switch_driver *type);
> +struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
>  
>  static inline void *ds_to_priv(struct dsa_switch *ds)
>  {
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 1df0a7c..b34d697 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -43,7 +43,7 @@ void unregister_switch_driver(struct dsa_switch_driver *drv)
>  EXPORT_SYMBOL_GPL(unregister_switch_driver);
>  
>  static struct dsa_switch_driver *
> -dsa_switch_probe(struct mii_bus *bus, int sw_addr, char **_name)
> +dsa_switch_probe(struct device *host_dev, int sw_addr, char **_name)
>  {
>  	struct dsa_switch_driver *ret;
>  	struct list_head *list;
> @@ -58,7 +58,7 @@ dsa_switch_probe(struct mii_bus *bus, int sw_addr, char **_name)
>  
>  		drv = list_entry(list, struct dsa_switch_driver, list);
>  
> -		name = drv->probe(bus, sw_addr);
> +		name = drv->probe(host_dev, sw_addr);
>  		if (name != NULL) {
>  			ret = drv;
>  			break;
> @@ -75,7 +75,7 @@ dsa_switch_probe(struct mii_bus *bus, int sw_addr, char **_name)
>  /* basic switch operations **************************************************/
>  static struct dsa_switch *
>  dsa_switch_setup(struct dsa_switch_tree *dst, int index,
> -		 struct device *parent, struct mii_bus *bus)
> +		 struct device *parent, struct device *host_dev)
>  {
>  	struct dsa_chip_data *pd = dst->pd->chip + index;
>  	struct dsa_switch_driver *drv;
> @@ -88,7 +88,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
>  	/*
>  	 * Probe for switch model.
>  	 */
> -	drv = dsa_switch_probe(bus, pd->sw_addr, &name);
> +	drv = dsa_switch_probe(host_dev, pd->sw_addr, &name);
>  	if (drv == NULL) {
>  		printk(KERN_ERR "%s[%d]: could not detect attached switch\n",
>  		       dst->master_netdev->name, index);
> @@ -109,8 +109,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
>  	ds->index = index;
>  	ds->pd = dst->pd->chip + index;
>  	ds->drv = drv;
> -	ds->master_mii_bus = bus;
> -
> +	ds->master_dev = host_dev;
>  
>  	/*
>  	 * Validate supplied switch configuration.
> @@ -285,7 +284,7 @@ static struct device *dev_find_class(struct device *parent, char *class)
>  	return device_find_child(parent, class, dev_is_class);
>  }
>  
> -static struct mii_bus *dev_to_mii_bus(struct device *dev)
> +struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>  {
>  	struct device *d;
>  
> @@ -301,6 +300,7 @@ static struct mii_bus *dev_to_mii_bus(struct device *dev)
>  
>  	return NULL;
>  }
> +EXPORT_SYMBOL_GPL(dsa_host_dev_to_mii_bus);
>  
>  static struct net_device *dev_to_net_device(struct device *dev)
>  {
> @@ -566,17 +566,9 @@ static int dsa_probe(struct platform_device *pdev)
>  	dst->cpu_port = -1;
>  
>  	for (i = 0; i < pd->nr_chips; i++) {
> -		struct mii_bus *bus;
>  		struct dsa_switch *ds;
>  
> -		bus = dev_to_mii_bus(pd->chip[i].mii_bus);
> -		if (bus == NULL) {
> -			printk(KERN_ERR "%s[%d]: no mii bus found for "
> -				"dsa switch\n", dev->name, i);
> -			continue;
> -		}
> -
> -		ds = dsa_switch_setup(dst, i, &pdev->dev, bus);
> +		ds = dsa_switch_setup(dst, i, &pdev->dev, pd->chip[i].host_dev);
>  		if (IS_ERR(ds)) {
>  			printk(KERN_ERR "%s[%d]: couldn't create dsa switch "
>  				"instance (error %ld)\n", dev->name, i,
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index e38a331..90c9689 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -44,7 +44,7 @@ void dsa_slave_mii_bus_init(struct dsa_switch *ds)
>  	ds->slave_mii_bus->write = dsa_slave_phy_write;
>  	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d:%.2x",
>  			ds->index, ds->pd->sw_addr);
> -	ds->slave_mii_bus->parent = &ds->master_mii_bus->dev;
> +	ds->slave_mii_bus->parent = ds->master_dev;
>  }
>  
>  
> 

^ permalink raw reply

* Re: [Patch net-next 1/5] net_sched: fix an allocation bug in tcindex_set_parms()
From: David Miller @ 2014-09-15 21:48 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, john.fastabend
In-Reply-To: <1410815210-6693-2-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 15 Sep 2014 14:06:46 -0700

> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

This seems trivially correct, applied, thanks.

^ permalink raw reply

* Re: [Patch net-next 2/5] net_sched: fix memory leak in cls_tcindex
From: David Miller @ 2014-09-15 21:49 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, john.fastabend
In-Reply-To: <1410815210-6693-3-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 15 Sep 2014 14:06:47 -0700

> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Please address John's feedback.

^ permalink raw reply

* Re: [Patch net-next 3/5] net_sched: fix suspicious RCU usage in tcindex_classify()
From: David Miller @ 2014-09-15 21:50 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, john.fastabend
In-Reply-To: <1410815210-6693-4-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 15 Sep 2014 14:06:48 -0700

> This patch fixes the following kernel warning:
> 
> [   44.805900] [ INFO: suspicious RCU usage. ]
> [   44.808946] 3.17.0-rc4+ #610 Not tainted
> [   44.811831] -------------------------------
> [   44.814873] net/sched/cls_tcindex.c:84 suspicious rcu_dereference_check() usage!
> 
> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied.

^ permalink raw reply

* Re: [Patch net-next 4/5] net_sched: use tcindex_filter_result_init()
From: David Miller @ 2014-09-15 21:51 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, john.fastabend
In-Reply-To: <1410815210-6693-5-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 15 Sep 2014 14:06:49 -0700

> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Clearly an equivalent transformation, applied, thanks.

^ permalink raw reply

* Re: [Patch net-next 5/5] net_sched: clean up tcindex_set_parms()
From: David Miller @ 2014-09-15 21:52 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, john.fastabend
In-Reply-To: <1410815210-6693-6-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 15 Sep 2014 14:06:50 -0700

> We can move the allocation down.
> 
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

I'll give John time to review this.

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: fix mii_bus to host_dev replacement
From: David Miller @ 2014-09-15 21:53 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, alexander.h.duyck
In-Reply-To: <1410817688-19397-1-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 15 Sep 2014 14:48:08 -0700

> dsa_of_probe() still used cd->mii_bus instead of cd->host_dev when
> building with CONFIG_OF=y. Fix this by making the replacement here as
> well.
> 
> Fixes: b4d2394d01b ("dsa: Replace mii_bus with a generic host device")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied, thanks for catching this.

^ permalink raw reply

* Re: [Patch net-next 2/5] net_sched: fix memory leak in cls_tcindex
From: Cong Wang @ 2014-09-15 22:15 UTC (permalink / raw)
  To: John Fastabend
  Cc: Linux Kernel Network Developers, john fastabend, David S. Miller
In-Reply-To: <54175D17.6030109@intel.com>

On Mon, Sep 15, 2014 at 2:41 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> On 09/15/2014 02:06 PM, Cong Wang wrote:
>> Fixes: commit 331b72922c5f58d48fd ("net: sched: RCU cls_tcindex")
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> ---
>>  net/sched/cls_tcindex.c | 13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
>> index a02ca72..16ec1ed 100644
>> --- a/net/sched/cls_tcindex.c
>> +++ b/net/sched/cls_tcindex.c
>> @@ -243,7 +243,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>>        */
>>       cp = kzalloc(sizeof(*cp), GFP_KERNEL);
>>       if (!cp)
>> -             return -ENOMEM;
>> +             goto errout;
>
> but you need to set 'err = -ENOMEM' then.

Yeah, I thought err is initialized to ENOMEM.

>
>>
>>       cp->mask = p->mask;
>>       cp->shift = p->shift;
>> @@ -257,6 +257,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>>                                     sizeof(*r) * cp->hash, GFP_KERNEL);
>>               if (!cp->perfect)
>>                       goto errout;
>> +             balloc = 1;
>
> Actually can we just get rid of the balloc here altogether and
> remove the checks in errout_alloc so that cp->perfect and cp->h
> are freed unconditionally? They should be NULL if they are not
> being used because of the kzalloc.
>

Hmm, but for cp->h which you don't duplicate if we free it we
free p->h too, which seems not expected. I think we probably
need to cp->h = kmemdup(p->h) as well.

^ 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