Netdev List
 help / color / mirror / Atom feed
* Re: qdisc/trafgen: Measuring effect of qdisc bulk dequeue, with trafgen
From: Jamal Hadi Salim @ 2014-09-19 11:57 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, netdev@vger.kernel.org
  Cc: David S. Miller, Tom Herbert, Hannes Frederic Sowa,
	Florian Westphal, Daniel Borkmann, Alexander Duyck,
	John Fastabend
In-Reply-To: <20140919123536.636fa226@redhat.com>

On 09/19/14 06:35, Jesper Dangaard Brouer wrote:
>
> This experiment were about finding the tipping-point, when bulking
> from the qdisc kicks in.  This is an artificial benchmark.
>
> This testing relates to my qdisc bulk dequeue patches:
>   http://thread.gmane.org/gmane.linux.network/328829/focus=328951
>
> My point have always been, we should only start bulking packets when
> really needed, I dislike attempts to delay TX in antisipation of
> packets arriving shortly (due to the added latency).  IMHO the qdisc
> layer seems the right place "see" when bulking makes sense.
>
> The reason behind this test is, there is two code paths in the qdisc
> layer.  1) when qdisc is empty we allow packet to directly call
> sch_direct_xmit(), 2) when qdisc contains packet we go through a more
> expensive process of enqueue, dequeue and possibly rescheduling a
> softirq.
>
> Thus, the cost when the qdisc kicks-in should be slightly higher.  My
> qdisc bulk dequeue patch, should help us actually getting faster in
> this case.  Below results (with dequeue bulking max 4 packets) show
> that, this was true, as expected the locking cost were reduced, giving
> us an actual speedup.
>
>
> Testing this tipping point is hard, but found an trafgen setup, that
> were just balancing on this tipping point, single CPU 1Gbit/s setup
> driver igb.
>

The feedback system is clearly very well oiled. Or is it now? ;->
Jesper, maybe you need to poke at system level as opposed to
microscopic lock level. The transmit path is essentially kicked by
tx softirq which is driven by rx path etc. And those guys work like
a clock pendulum.
To busy that sucker, You may be able to get more luck with
forwarding kind of traffic.
Funnel traffic from many nic ports tied to different CPUs to one egress
port.
Some coffee helped me remember i actually surrendered that it can be
done at all in netconf 2011[1] but please let me not poison your
thinking - you may find otherwise.

cheers,
jamal

http://vger.kernel.org/netconf2011_slides/jamal_netconf2011.pdf slide 12

^ permalink raw reply

* Re: [PATCH net-next v4 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-19 11:06 UTC (permalink / raw)
  To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	David Miller
  Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <541BFEA4.9080702@redhat.com>

On 09/19/2014 12:00 PM, Nikolay Aleksandrov wrote:
> On 09/18/2014 11:53 PM, Mahesh Bandewar wrote:
>> Earlier change to use usable slave array for TLB mode had an additional
>> performance advantage. So extending the same logic to all other modes
>> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
>> Also consolidating this with the earlier TLB change.
>>
>> The main idea is to build the usable slaves array in the control path
>> and use that array for slave selection during xmit operation.
>>
>> Measured performance in a setup with a bond of 4x1G NICs with 200
>> instances of netperf for the modes involved (3ad, xor, tlb)
>> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>>
>> Mode        TPS-Before   TPS-After
>>
>> 802.3ad   : 468,694      493,101
>> TLB (lb=0): 392,583      392,965
>> XOR       : 475,696      484,517
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>> v1:
>>   (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
>>       the slave that need to be removed.
>>   (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>>       transition gracefully.
>>   (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
>>       failure.
>>   (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
>>       will populate the array even if these parameters are not used.
>>   (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>> v2:
>>   (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>>   (b) Slave link-events now refresh array for all these modes.
>>   (c) Moved free-array call from bond_close() to bond_uninit().
>> v3:
>>   (a) Fixed null pointer dereference.
>>   (b) Removed bond->lock lockdep dependency.
>> v4:
>>   (a) Made to changes to comply with Nikolay's locking changes
>>   (b) Added a work-queue to refresh slave-array when RTNL is not held
>>   (c) Array refresh happens ONLY with RTNL now.
>>   (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
>>
<<<snip>>>
>> @@ -3839,6 +4003,7 @@ static void bond_uninit(struct net_device *bond_dev)
>>  	struct bonding *bond = netdev_priv(bond_dev);
>>  	struct list_head *iter;
>>  	struct slave *slave;
>> +	struct bond_up_slave *arr;
>>  
>>  	bond_netpoll_cleanup(bond_dev);
>>  
>> @@ -3847,6 +4012,12 @@ static void bond_uninit(struct net_device *bond_dev)
>>  		__bond_release_one(bond_dev, slave->dev, true);
>>  	netdev_info(bond_dev, "Released all slaves\n");
>>  
Sorry but I just spotted a major problem, bond_3ad_unbind_slave() (called
from __bond_release_one) calls ad_agg_selection_logic() which can re-arm
the slave_arr work after it's supposed to be stopped here (i.e. the bond
device has been closed so all works should've been stopped) so we might
leak memory and access freed memory after all since it'll keep
re-scheduling itself until it can acquire rtnl which is after the bond
device has been destroyed.

>> +	arr = rtnl_dereference(bond->slave_arr);
>> +	if (arr) {
>> +		kfree_rcu(arr, rcu);
>> +		RCU_INIT_POINTER(bond->slave_arr, NULL);
>> +	}
>> +
>>  	list_del(&bond->bond_list);
>>  
>>  	bond_debug_unregister(bond);
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 98dc0d7ad731..4635b175256a 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -177,6 +177,12 @@ struct slave {
>>  	struct kobject kobj;
>>  };
>>  
>> +struct bond_up_slave {
>> +	unsigned int	count;
>> +	struct rcu_head rcu;
>> +	struct slave	*arr[0];
>> +};
>> +
>>  /*
>>   * Link pseudo-state only used internally by monitors
>>   */
>> @@ -191,6 +197,7 @@ struct bonding {
>>  	struct   slave __rcu *curr_active_slave;
>>  	struct   slave __rcu *current_arp_slave;
>>  	struct   slave __rcu *primary_slave;
>> +	struct   bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
>>  	bool     force_primary;
>>  	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
>>  	int     (*recv_probe)(const struct sk_buff *, struct bonding *,
>> @@ -220,6 +227,7 @@ struct bonding {
>>  	struct   delayed_work alb_work;
>>  	struct   delayed_work ad_work;
>>  	struct   delayed_work mcast_work;
>> +	struct   delayed_work slave_arr_work;
>>  #ifdef CONFIG_DEBUG_FS
>>  	/* debugging support via debugfs */
>>  	struct	 dentry *debug_dir;
>> @@ -531,6 +539,8 @@ const char *bond_slave_link_status(s8 link);
>>  struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
>>  					      struct net_device *end_dev,
>>  					      int level);
>> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
>> +void bond_slave_arr_work_rearm(struct bonding *bond);
>>  
>>  #ifdef CONFIG_PROC_FS
>>  void bond_create_proc_entry(struct bonding *bond);
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [net-next PATCH 00/29] Add support for the Intel FM10000 Ethernet Switch Host Interface
From: Jamal Hadi Salim @ 2014-09-19 10:57 UTC (permalink / raw)
  To: Alexander Duyck, davem
  Cc: nhorman, netdev, john.fastabend, matthew.vick, jeffrey.t.kirsher,
	sassmann
In-Reply-To: <20140918223242.10373.27403.stgit@ahduyck-bv4.jf.intel.com>

On 09/18/14 18:35, Alexander Duyck wrote:
> This patch series adds support for the FM10000 Ethernet switch host
> interface.  The Intel FM10000 Ethernet Switch is a 48-port Ethernet switch
> supporting both Ethernet ports and PCI Express host interfaces.  The fm10k
> driver provides support for the host interface portion of the switch, both
> PF and VF.
>
> As the host interfaces are directly connected to the switch this results in
> some significant differences versus a standard network driver.  For example
> there is no PHY or MII on the device.  Since packets are delivered directly
> from the switch to the host interface these are unnecessary.  Otherwise most
> of the functionality is very similar to our other network drivers such as
> ixgbe or igb.  For example we support all the standard network offloads,
> jumbo frames, SR-IOV (64 VFS), PTP, and some VXLAN and NVGRE offloads.
>

First off Kudos to the intel folks. Hopefully this is one of many such
drivers coming - and lets hope this is as landscape-changing as the
e1000 was ;-> Broadcom smell that Tim Horton's coffee.

You described this as a switch but then expose it as a glorified nic.
Assuming more goodstuff(tm) coming?

cheers,
jamal

^ permalink raw reply

* qdisc/UDP_STREAM: measuring effect of qdisc bulk dequeue
From: Jesper Dangaard Brouer @ 2014-09-19 10:44 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: brouer, netdev@vger.kernel.org, David S. Miller, Tom Herbert,
	Hannes Frederic Sowa, Florian Westphal, Daniel Borkmann,
	Jamal Hadi Salim, Alexander Duyck, John Fastabend
In-Reply-To: <20140919123536.636fa226@redhat.com>


On Fri, 19 Sep 2014 12:35:36 +0200
Jesper Dangaard Brouer <jbrouer@redhat.com> wrote:
 
> This testing relates to my qdisc bulk dequeue patches:
>  http://thread.gmane.org/gmane.linux.network/328829/focus=328951


I will quickly followup, with a more real-life use-case for qdisc layer
dequeue bulking (as Eric dislikes my artificial benchmarks ;-)).

Using UDP_STREAM on 1Gbit/s driver igb, I can show that the
_raw_spin_lock calls are reduced with approx 3%, when enabling
bulking of just 2 packets.

This test can only demonstrates a CPU usage reduction, as the
throughput is already at maximum link (bandwidth) capacity.


Notice netperf option "-m 1472" which makes sure we are not sending
UDP IP-fragments::

 netperf -H 192.168.111.2 -t UDP_STREAM -l 120 -- -m 1472


Results from perf diff::

 # Command: perf diff
 # Event 'cycles'
 # Baseline  Delta    Symbol
 # no-bulk   bulk(1)
 # ........  .......  .........................................
 #
     7.05%   -3.03%  [k] _raw_spin_lock
     6.34%   +0.23%  [k] copy_user_enhanced_fast_string
     6.30%   +0.26%  [k] fib_table_lookup
     3.03%   +0.01%  [k] __slab_free
     3.00%   +0.08%  [k] intel_idle
     2.49%   +0.05%  [k] sock_alloc_send_pskb
     2.31%   +0.30%  netperf  [.] send_omni_inner
     2.12%   +0.12%  netperf  [.] send_data
     2.11%   +0.10%  [k] udp_sendmsg
     1.96%   +0.02%  [k] __ip_append_data
     1.48%   -0.01%  [k] __alloc_skb
     1.46%   +0.07%  [k] __mkroute_output
     1.34%   +0.05%  [k] __ip_select_ident
     1.29%   +0.03%  [k] check_leaf
     1.27%   +0.09%  [k] __skb_get_hash

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer


Settings::

 export N=0; sudo sh -c "echo $N > /proc/sys/net/core/qdisc_bulk_dequeue_limit"; \
 grep -H . /proc/sys/net/core/qdisc_bulk_dequeue_limit
 /proc/sys/net/core/qdisc_bulk_dequeue_limit:0

 export N=1; sudo sh -c "echo $N > /proc/sys/net/core/qdisc_bulk_dequeue_limit"; \
 grep -H . /proc/sys/net/core/qdisc_bulk_dequeue_limit
 /proc/sys/net/core/qdisc_bulk_dequeue_limit:1

^ permalink raw reply

* [patch -next] r8169: fix an if condition
From: Dan Carpenter @ 2014-09-19 10:40 UTC (permalink / raw)
  To: Realtek linux nic maintainers, hayeswang; +Cc: netdev, kernel-janitors

There is an extra semi-colon so __rtl8169_set_features() is called every
time.

Fixes: 929a031dfd62 ('r8169: adjust __rtl8169_set_features')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 3027f4a..1d81238 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1882,7 +1882,7 @@ static int rtl8169_set_features(struct net_device *dev,
 	features &= NETIF_F_RXALL | NETIF_F_RXCSUM | NETIF_F_HW_VLAN_CTAG_RX;
 
 	rtl_lock_work(tp);
-	if (features ^ dev->features);
+	if (features ^ dev->features)
 		__rtl8169_set_features(dev, features);
 	rtl_unlock_work(tp);
 

^ permalink raw reply related

* qdisc/trafgen: Measuring effect of qdisc bulk dequeue, with trafgen
From: Jesper Dangaard Brouer @ 2014-09-19 10:35 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: David S. Miller, Tom Herbert, Hannes Frederic Sowa,
	Florian Westphal, Daniel Borkmann, Jamal Hadi Salim,
	Alexander Duyck, John Fastabend


This experiment were about finding the tipping-point, when bulking
from the qdisc kicks in.  This is an artificial benchmark.

This testing relates to my qdisc bulk dequeue patches:
 http://thread.gmane.org/gmane.linux.network/328829/focus=328951

My point have always been, we should only start bulking packets when
really needed, I dislike attempts to delay TX in antisipation of
packets arriving shortly (due to the added latency).  IMHO the qdisc
layer seems the right place "see" when bulking makes sense.

The reason behind this test is, there is two code paths in the qdisc
layer.  1) when qdisc is empty we allow packet to directly call
sch_direct_xmit(), 2) when qdisc contains packet we go through a more
expensive process of enqueue, dequeue and possibly rescheduling a
softirq.

Thus, the cost when the qdisc kicks-in should be slightly higher.  My
qdisc bulk dequeue patch, should help us actually getting faster in
this case.  Below results (with dequeue bulking max 4 packets) show
that, this was true, as expected the locking cost were reduced, giving
us an actual speedup.


Testing this tipping point is hard, but found an trafgen setup, that
were just balancing on this tipping point, single CPU 1Gbit/s setup
driver igb.

 # trafgen --cpp --dev eth1 --conf udp_example02_const.trafgen -V  --qdisc-path -t0 --cpus 1

With this specific trafgen setup, I could show that: when qdisc queue
was empty I could not hit wirespeed 1G.

* instant rx:0 tx:1423314 pps n:60 average: rx:0 tx:1423454 pps
  (instant variation TX -0.069 ns (min:-0.707 max:0.392) RX 0.000 ns)

Perf showed the top#1 (13.49%) item was _raw_spin_lock called 81.32%
by sch_direct_xmit() and 16.92% by __dev_queue_xmit()

Sometimes by itself trafgen, creates a qdisc backlog, and _then_ the
qdisc bulking kicks-in. Resulting in full 1G wirespeed.

* instant rx:0 tx:1489324 pps n:29 average: rx:0 tx:1489263 pps
 (instant variation TX 0.028 ns (min:-0.040 max:0.028) RX 0.000 ns)
* Diff: (1/1423314*10^9)-(1/1489324*10^9) = 31ns

The process could be triggered by e.g. starting a netperf on another
CPU, cause qdisc to backlog for the trafgen-qdisc, when stopping the
netperf the trafgen-qdisc keep backlogged (at least for a while), and
NOW it hits wirespeed 1G.

Perf record/diff showed exactly what was expect: The _raw_spin_lock
was now top#3 with 6.09% (down with 7.20%).  The distribution of
callers (of _raw_spin_lock) have changed (and sort-of swapped) with
58.64% by __dev_queue_xmit() and only 33.54% by sch_direct_xmit().

# perf diff
# Baseline    Delta  Symbol
# no-bulk    bulk(4)
# ........  .......  .........................
    13.55%   -7.20%  [k] _raw_spin_lock
    11.88%   -0.06%  [k] sock_alloc_send_pskb
     6.24%   +0.12%  [k] packet_snd
     3.66%   -1.34%  [k] igb_tx_map
     3.58%   +0.07%  [k] __alloc_skb
     2.86%   +0.16%  [k] __dev_queue_xmit

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [PATCH net-next 11/12] be2net: remove multiple assignments on a single line
From: Sathya Perla @ 2014-09-19 10:17 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch removes multiple assignments on a single line as warned
by checkpatch.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c    |    8 ++++----
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    6 ++++--
 drivers/net/ethernet/emulex/benet/be_main.c    |    6 ++++--
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 48f4be3..58e7eff 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1910,8 +1910,8 @@ int be_cmd_rx_filter(struct be_adapter *adapter, u32 flags, u32 value)
 					    BE_IF_FLAGS_VLAN_PROMISCUOUS |
 					    BE_IF_FLAGS_MCAST_PROMISCUOUS);
 	} else if (flags & IFF_ALLMULTI) {
-		req->if_flags_mask = req->if_flags =
-				cpu_to_le32(BE_IF_FLAGS_MCAST_PROMISCUOUS);
+		req->if_flags_mask = cpu_to_le32(BE_IF_FLAGS_MCAST_PROMISCUOUS);
+		req->if_flags =	cpu_to_le32(BE_IF_FLAGS_MCAST_PROMISCUOUS);
 	} else if (flags & BE_FLAGS_VLAN_PROMISC) {
 		req->if_flags_mask = cpu_to_le32(BE_IF_FLAGS_VLAN_PROMISCUOUS);
 
@@ -1922,8 +1922,8 @@ int be_cmd_rx_filter(struct be_adapter *adapter, u32 flags, u32 value)
 		struct netdev_hw_addr *ha;
 		int i = 0;
 
-		req->if_flags_mask = req->if_flags =
-				cpu_to_le32(BE_IF_FLAGS_MULTICAST);
+		req->if_flags_mask = cpu_to_le32(BE_IF_FLAGS_MULTICAST);
+		req->if_flags =	cpu_to_le32(BE_IF_FLAGS_MULTICAST);
 
 		/* Reset mcast promisc mode if already set by setting mask
 		 * and not setting flags field
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 42b10f9..240fb46 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -684,8 +684,10 @@ static void be_get_ringparam(struct net_device *netdev,
 {
 	struct be_adapter *adapter = netdev_priv(netdev);
 
-	ring->rx_max_pending = ring->rx_pending = adapter->rx_obj[0].q.len;
-	ring->tx_max_pending = ring->tx_pending = adapter->tx_obj[0].q.len;
+	ring->rx_max_pending = adapter->rx_obj[0].q.len;
+	ring->rx_pending = adapter->rx_obj[0].q.len;
+	ring->tx_max_pending = adapter->tx_obj[0].q.len;
+	ring->tx_pending = adapter->tx_obj[0].q.len;
 }
 
 static void
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 0bed42b..78f1de6 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2061,7 +2061,8 @@ static void be_rx_cq_clean(struct be_rx_obj *rxo)
 		memset(page_info, 0, sizeof(*page_info));
 	}
 	BUG_ON(atomic_read(&rxq->used));
-	rxq->tail = rxq->head = 0;
+	rxq->tail = 0;
+	rxq->head = 0;
 }
 
 static void be_tx_compl_clean(struct be_adapter *adapter)
@@ -4923,7 +4924,8 @@ static int be_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id)
 
 	INIT_DELAYED_WORK(&adapter->work, be_worker);
 	INIT_DELAYED_WORK(&adapter->func_recovery_work, be_func_recovery_task);
-	adapter->rx_fc = adapter->tx_fc = true;
+	adapter->rx_fc = true;
+	adapter->tx_fc = true;
 
 	status = be_setup(adapter);
 	if (status)
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 09/12] be2net: remove unnecessary blank lines after an open brace
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch fixes checkpatch warnings about blank lines after an open brace '{'.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    3 ---
 drivers/net/ethernet/emulex/benet/be_main.c |    2 --
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index bc47d17..9ec09a8 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -209,7 +209,6 @@ static int be_mcc_compl_process(struct be_adapter *adapter,
 
 	if (base_status != MCC_STATUS_SUCCESS &&
 	    !be_skip_err_log(opcode, base_status, addl_status)) {
-
 		if (base_status == MCC_STATUS_UNAUTHORIZED_REQUEST) {
 			dev_warn(&adapter->pdev->dev,
 				 "VF is not privileged to issue opcode %d-%d\n",
@@ -1522,7 +1521,6 @@ err:
 int lancer_cmd_get_pport_stats(struct be_adapter *adapter,
 			       struct be_dma_mem *nonemb_cmd)
 {
-
 	struct be_mcc_wrb *wrb;
 	struct lancer_cmd_req_pport_stats *req;
 	int status = 0;
@@ -3038,7 +3036,6 @@ out:
 int be_cmd_get_active_mac(struct be_adapter *adapter, u32 curr_pmac_id,
 			  u8 *mac, u32 if_handle, bool active, u32 domain)
 {
-
 	if (!active)
 		be_cmd_get_mac_from_list(adapter, mac, &active, &curr_pmac_id,
 					 if_handle, domain);
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 31b5fdc..7e80527 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -493,7 +493,6 @@ static void populate_be_v2_stats(struct be_adapter *adapter)
 
 static void populate_lancer_stats(struct be_adapter *adapter)
 {
-
 	struct be_drv_stats *drvs = &adapter->drv_stats;
 	struct lancer_pport_stats *pport_stats = pport_stats_from_cmd(adapter);
 
@@ -4741,7 +4740,6 @@ static void be_func_recovery_task(struct work_struct *work)
 	be_detect_error(adapter);
 
 	if (adapter->hw_error && lancer_chip(adapter)) {
-
 		rtnl_lock();
 		netif_device_detach(adapter->netdev);
 		rtnl_unlock();
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 12/12] be2net: fix alignment on line wrap
From: Sathya Perla @ 2014-09-19 10:17 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch fixes alignment whereever it doesn't match the open parenthesis
alignment.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c    |    9 +++++----
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    6 +++---
 drivers/net/ethernet/emulex/benet/be_main.c    |    6 +++---
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 58e7eff..fead5c6 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1034,7 +1034,8 @@ int be_cmd_pmac_del(struct be_adapter *adapter, u32 if_id, int pmac_id, u32 dom)
 	req = embedded_payload(wrb);
 
 	be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
-		OPCODE_COMMON_NTWK_PMAC_DEL, sizeof(*req), wrb, NULL);
+			       OPCODE_COMMON_NTWK_PMAC_DEL, sizeof(*req),
+			       wrb, NULL);
 
 	req->hdr.domain = dom;
 	req->if_id = cpu_to_le32(if_id);
@@ -1709,7 +1710,7 @@ int be_cmd_get_regs(struct be_adapter *adapter, u32 buf_len, void *buf)
 					      &get_fat_cmd.dma);
 	if (!get_fat_cmd.va) {
 		dev_err(&adapter->pdev->dev,
-		"Memory allocation failure while retrieving FAT data\n");
+			"Memory allocation failure while reading FAT data\n");
 		return -ENOMEM;
 	}
 
@@ -2471,7 +2472,7 @@ err_unlock:
 }
 
 int be_cmd_get_flash_crc(struct be_adapter *adapter, u8 *flashed_crc,
-			  u16 optype, int offset)
+			 u16 optype, int offset)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_read_flash_crc *req;
@@ -2645,7 +2646,7 @@ int be_cmd_ddr_dma_test(struct be_adapter *adapter, u64 pattern,
 
 		resp = cmd->va;
 		if ((memcmp(resp->rcv_buff, req->snd_buff, byte_cnt) != 0) ||
-				resp->snd_err) {
+		    resp->snd_err) {
 			status = -1;
 		}
 	}
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 240fb46..e42a791 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -277,7 +277,7 @@ static int lancer_cmd_read_file(struct be_adapter *adapter, u8 *file_name,
 
 	while ((total_read_len < buf_len) && !eof) {
 		chunk_size = min_t(u32, (buf_len - total_read_len),
-				LANCER_READ_FILE_CHUNK);
+				   LANCER_READ_FILE_CHUNK);
 		chunk_size = ALIGN(chunk_size, 4);
 		status = lancer_cmd_read_object(adapter, &read_cmd, chunk_size,
 						total_read_len, file_name,
@@ -1213,8 +1213,8 @@ static int be_set_rxfh(struct net_device *netdev, const u32 *indir,
 		hkey =  adapter->rss_info.rss_hkey;
 
 	rc = be_cmd_rss_config(adapter, rsstable,
-			adapter->rss_info.rss_flags,
-			RSS_INDIR_TABLE_LEN, hkey);
+			       adapter->rss_info.rss_flags,
+			       RSS_INDIR_TABLE_LEN, hkey);
 	if (rc) {
 		adapter->rss_info.rss_flags = RSS_ENABLE_NONE;
 		return -EIO;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 78f1de6..9a18e79 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -982,8 +982,8 @@ static struct sk_buff *be_lancer_xmit_workarounds(struct be_adapter *adapter,
 	 * skip HW tagging is not enabled by FW.
 	 */
 	if (unlikely(be_ipv6_tx_stall_chk(adapter, skb) &&
-	    (adapter->pvid || adapter->qnq_vid) &&
-	    !qnq_async_evt_rcvd(adapter)))
+		     (adapter->pvid || adapter->qnq_vid) &&
+		     !qnq_async_evt_rcvd(adapter)))
 		goto tx_drop;
 
 	/* Manual VLAN tag insertion to prevent:
@@ -4777,7 +4777,7 @@ static void be_worker(struct work_struct *work)
 	if (!adapter->stats_cmd_sent) {
 		if (lancer_chip(adapter))
 			lancer_cmd_get_pport_stats(adapter,
-						&adapter->stats_cmd);
+						   &adapter->stats_cmd);
 		else
 			be_cmd_get_stats(adapter, &adapter->stats_cmd);
 	}
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 07/12] be2net: remove multiple blank lines
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch removes multiple blank lines in the driver as per checkpatch
warnings.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c    |    1 -
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    1 -
 drivers/net/ethernet/emulex/benet/be_main.c    |    2 --
 3 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 8dd4975..bc47d17 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -676,7 +676,6 @@ int be_fw_wait_ready(struct be_adapter *adapter)
 	return -1;
 }
 
-
 static inline struct be_sge *nonembedded_sgl(struct be_mcc_wrb *wrb)
 {
 	return &wrb->payload.sgl[0];
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index eaf7003..9a2a72b 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -333,7 +333,6 @@ static int be_get_coalesce(struct net_device *netdev,
 	struct be_adapter *adapter = netdev_priv(netdev);
 	struct be_aic_obj *aic = &adapter->aic_obj[0];
 
-
 	et->rx_coalesce_usecs = aic->prev_eqd;
 	et->rx_coalesce_usecs_high = aic->max_eqd;
 	et->rx_coalesce_usecs_low = aic->min_eqd;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index fbab943..8095c83 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -122,7 +122,6 @@ static const char * const ue_status_hi_desc[] = {
 	"Unknown"
 };
 
-
 static void be_queue_free(struct be_adapter *adapter, struct be_queue_info *q)
 {
 	struct be_dma_mem *mem = &q->dma_mem;
@@ -1490,7 +1489,6 @@ static void be_eqd_update(struct be_adapter *adapter)
 			tx_pkts = txo->stats.tx_reqs;
 		} while (u64_stats_fetch_retry_irq(&txo->stats.sync, start));
 
-
 		/* Skip, if wrapped around or first calculation */
 		now = jiffies;
 		if (!aic->jiffies || time_before(now, aic->jiffies) ||
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 05/12] be2net: remove return statements for void functions
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    2 --
 drivers/net/ethernet/emulex/benet/be_main.c    |    3 ---
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index ec66f64..32c61f3 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -975,8 +975,6 @@ static void be_set_msg_level(struct net_device *netdev, u32 level)
 						FW_LOG_LEVEL_DEFAULT :
 						FW_LOG_LEVEL_FATAL);
 	adapter->msg_enable = level;
-
-	return;
 }
 
 static u64 be_get_rss_hash_opts(struct be_adapter *adapter, u64 flow_type)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index beebab6..96be231 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3716,8 +3716,6 @@ static void be_netpoll(struct net_device *netdev)
 		be_eq_notify(eqo->adapter, eqo->q.id, false, true, 0);
 		napi_schedule(&eqo->napi);
 	}
-
-	return;
 }
 #endif
 
@@ -4395,7 +4393,6 @@ static void be_add_vxlan_port(struct net_device *netdev, sa_family_t sa_family,
 	return;
 err:
 	be_disable_vxlan_offloads(adapter);
-	return;
 }
 
 static void be_del_vxlan_port(struct net_device *netdev, sa_family_t sa_family,
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 10/12] be2net: remove space after typecasts
From: Sathya Perla @ 2014-09-19 10:17 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch removes unnecessary spaces after typecasts as per checkpatch warnings.
Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    8 ++++----
 drivers/net/ethernet/emulex/benet/be_main.c |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 9ec09a8..48f4be3 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -316,7 +316,7 @@ static void be_async_dbg_evt_process(struct be_adapter *adapter,
 				     struct be_mcc_compl *cmp)
 {
 	u8 event_type = 0;
-	struct be_async_event_qnq *evt = (struct be_async_event_qnq *) cmp;
+	struct be_async_event_qnq *evt = (struct be_async_event_qnq *)cmp;
 
 	event_type = (cmp->flags >> ASYNC_EVENT_TYPE_SHIFT) &
 			ASYNC_EVENT_TYPE_MASK;
@@ -956,7 +956,7 @@ int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
 	if (permanent) {
 		req->permanent = 1;
 	} else {
-		req->if_id = cpu_to_le16((u16) if_handle);
+		req->if_id = cpu_to_le16((u16)if_handle);
 		req->pmac_id = cpu_to_le32(pmac_id);
 		req->permanent = 0;
 	}
@@ -3280,7 +3280,7 @@ int be_cmd_get_acpi_wol_cap(struct be_adapter *adapter)
 	if (!status) {
 		struct be_cmd_resp_acpi_wol_magic_config_v1 *resp;
 
-		resp = (struct be_cmd_resp_acpi_wol_magic_config_v1 *) cmd.va;
+		resp = (struct be_cmd_resp_acpi_wol_magic_config_v1 *)cmd.va;
 
 		adapter->wol_cap = resp->wol_settings;
 		if (adapter->wol_cap & BE_WOL_CAP)
@@ -4127,7 +4127,7 @@ int be_roce_mcc_cmd(void *netdev_handle, void *wrb_payload,
 {
 	struct be_adapter *adapter = netdev_priv(netdev_handle);
 	struct be_mcc_wrb *wrb;
-	struct be_cmd_req_hdr *hdr = (struct be_cmd_req_hdr *) wrb_payload;
+	struct be_cmd_req_hdr *hdr = (struct be_cmd_req_hdr *)wrb_payload;
 	struct be_cmd_req_hdr *req;
 	struct be_cmd_resp_hdr *resp;
 	int status;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 7e80527..0bed42b 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -918,7 +918,7 @@ static bool be_ipv6_exthdr_check(struct sk_buff *skb)
 		if (ip6h->nexthdr != NEXTHDR_TCP &&
 		    ip6h->nexthdr != NEXTHDR_UDP) {
 			struct ipv6_opt_hdr *ehdr =
-				(struct ipv6_opt_hdr *) (skb->data + offset);
+				(struct ipv6_opt_hdr *)(skb->data + offset);
 
 			/* offending pkt: 2nd byte following IPv6 hdr is 0xff */
 			if (ehdr->hdrlen == 0xff)
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 08/12] be2net: insert a blank line after function/struct//enum definitions
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch inserts a blank line after function/struct/union/enum definitions
as per checkpatch warnings.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    3 +++
 drivers/net/ethernet/emulex/benet/be_main.c    |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 9a2a72b..42b10f9 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -130,6 +130,7 @@ static const struct be_ethtool_stat et_stats[] = {
 	{DRVSTAT_INFO(roce_drops_payload_len)},
 	{DRVSTAT_INFO(roce_drops_crc)}
 };
+
 #define ETHTOOL_STATS_NUM ARRAY_SIZE(et_stats)
 
 /* Stats related to multi RX queues: get_stats routine assumes bytes, pkts
@@ -152,6 +153,7 @@ static const struct be_ethtool_stat et_rx_stats[] = {
 	 */
 	{DRVSTAT_RX_INFO(rx_drops_no_frags)}
 };
+
 #define ETHTOOL_RXSTATS_NUM (ARRAY_SIZE(et_rx_stats))
 
 /* Stats related to multi TX queues: get_stats routine assumes compl is the
@@ -200,6 +202,7 @@ static const struct be_ethtool_stat et_tx_stats[] = {
 	/* Pkts dropped in the driver's transmit path */
 	{DRVSTAT_TX_INFO(tx_drv_drops)}
 };
+
 #define ETHTOOL_TXSTATS_NUM (ARRAY_SIZE(et_tx_stats))
 
 static const char et_self_tests[][ETH_GSTRING_LEN] = {
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 8095c83..31b5fdc 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -86,6 +86,7 @@ static const char * const ue_status_low_desc[] = {
 	"JTAG ",
 	"MPU_INTPEND "
 };
+
 /* UE Status High CSR */
 static const char * const ue_status_hi_desc[] = {
 	"LPCMEMHOST",
@@ -1424,6 +1425,7 @@ err:
 		max_tx_rate, vf);
 	return be_cmd_status(status);
 }
+
 static int be_set_vf_link_state(struct net_device *netdev, int vf,
 				int link_state)
 {
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 03/12] be2net: add speed reporting for 40G/KR interface
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch adds speed reporting via ethtool for 40Gbps KR4 interface
on the Skyhawk-R chip.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 43b5595..8297824 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -538,6 +538,13 @@ static u32 convert_to_et_setting(struct be_adapter *adapter, u32 if_speeds)
 		val |= SUPPORTED_Backplane |
 				SUPPORTED_10000baseKR_Full;
 		break;
+	case PHY_TYPE_KR4_40GB:
+		val |= SUPPORTED_Backplane;
+		if (if_speeds & BE_SUPPORTED_SPEED_10GBPS)
+			val |= SUPPORTED_10000baseKR_Full;
+		if (if_speeds & BE_SUPPORTED_SPEED_40GBPS)
+			val |= SUPPORTED_40000baseKR4_Full;
+		break;
 	case PHY_TYPE_QSFP:
 		if (if_speeds & BE_SUPPORTED_SPEED_40GBPS) {
 			switch (adapter->phy.cable_type) {
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 01/12] be2net: fix a sparse warning in be_cmd_modify_eqd()
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch fixes a sparse warning about missing static declaration that was
introduced by the following commit:

fixes: 936767039cdf ("be2net: send a max of 8 EQs to be_cmd_modify_eqd() on Lancer")

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index cd213d9..2874c0b 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1783,8 +1783,8 @@ err:
 /* set the EQ delay interval of an EQ to specified value
  * Uses async mcc
  */
-int __be_cmd_modify_eqd(struct be_adapter *adapter, struct be_set_eqd *set_eqd,
-			int num)
+static int __be_cmd_modify_eqd(struct be_adapter *adapter,
+			       struct be_set_eqd *set_eqd, int num)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_req_modify_eq_delay *req;
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 06/12] be2net: add blank line after declarations
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Kalesh AP <kalesh.purayil@emulex.com>

This patch fixes checkpatch warnings in be2net by adding a blank line
between declaration and code blocks.

Signed-off-by: Kalesh AP <kalesh.purayil@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c    |   27 ++++++++++++++++++++++++
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    1 +
 drivers/net/ethernet/emulex/benet/be_main.c    |    9 ++++++++
 drivers/net/ethernet/emulex/benet/be_roce.c    |    1 +
 4 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 2874c0b..8dd4975 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -593,6 +593,7 @@ static int lancer_wait_ready(struct be_adapter *adapter)
 static bool lancer_provisioning_error(struct be_adapter *adapter)
 {
 	u32 sliport_status = 0, sliport_err1 = 0, sliport_err2 = 0;
+
 	sliport_status = ioread32(adapter->db + SLIPORT_STATUS_OFFSET);
 	if (sliport_status & SLIPORT_STATUS_ERR_MASK) {
 		sliport_err1 = ioread32(adapter->db + SLIPORT_ERROR1_OFFSET);
@@ -922,6 +923,7 @@ int be_cmd_eq_create(struct be_adapter *adapter, struct be_eq_obj *eqo)
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_eq_create *resp = embedded_payload(wrb);
+
 		eqo->q.id = le16_to_cpu(resp->eq_id);
 		eqo->msix_idx =
 			(ver == 2) ? le16_to_cpu(resp->msix_idx) : eqo->idx;
@@ -964,6 +966,7 @@ int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_mac_query *resp = embedded_payload(wrb);
+
 		memcpy(mac_addr, resp->mac.addr, ETH_ALEN);
 	}
 
@@ -1000,6 +1003,7 @@ int be_cmd_pmac_add(struct be_adapter *adapter, u8 *mac_addr,
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_pmac_add *resp = embedded_payload(wrb);
+
 		*pmac_id = le32_to_cpu(resp->pmac_id);
 	}
 
@@ -1104,6 +1108,7 @@ int be_cmd_cq_create(struct be_adapter *adapter, struct be_queue_info *cq,
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_cq_create *resp = embedded_payload(wrb);
+
 		cq->id = le16_to_cpu(resp->cq_id);
 		cq->created = true;
 	}
@@ -1116,6 +1121,7 @@ int be_cmd_cq_create(struct be_adapter *adapter, struct be_queue_info *cq,
 static u32 be_encoded_q_len(int q_len)
 {
 	u32 len_encoded = fls(q_len); /* log2(len) + 1 */
+
 	if (len_encoded == 16)
 		len_encoded = 0;
 	return len_encoded;
@@ -1171,6 +1177,7 @@ static int be_cmd_mccq_ext_create(struct be_adapter *adapter,
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_mcc_create *resp = embedded_payload(wrb);
+
 		mccq->id = le16_to_cpu(resp->id);
 		mccq->created = true;
 	}
@@ -1214,6 +1221,7 @@ static int be_cmd_mccq_org_create(struct be_adapter *adapter,
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_mcc_create *resp = embedded_payload(wrb);
+
 		mccq->id = le16_to_cpu(resp->id);
 		mccq->created = true;
 	}
@@ -1272,6 +1280,7 @@ int be_cmd_txq_create(struct be_adapter *adapter, struct be_tx_obj *txo)
 	status = be_cmd_notify_wait(adapter, &wrb);
 	if (!status) {
 		struct be_cmd_resp_eth_tx_create *resp = embedded_payload(&wrb);
+
 		txq->id = le16_to_cpu(resp->cid);
 		if (ver == 2)
 			txo->db_offset = le32_to_cpu(resp->db_offset);
@@ -1316,6 +1325,7 @@ int be_cmd_rxq_create(struct be_adapter *adapter,
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_eth_rx_create *resp = embedded_payload(wrb);
+
 		rxq->id = le16_to_cpu(resp->id);
 		rxq->created = true;
 		*rss_id = resp->rss_id;
@@ -1429,6 +1439,7 @@ int be_cmd_if_create(struct be_adapter *adapter, u32 cap_flags, u32 en_flags,
 	status = be_cmd_notify_wait(adapter, &wrb);
 	if (!status) {
 		struct be_cmd_resp_if_create *resp = embedded_payload(&wrb);
+
 		*if_handle = le32_to_cpu(resp->interface_id);
 
 		/* Hack to retrieve VF's pmac-id on BE3 */
@@ -1603,6 +1614,7 @@ int be_cmd_link_status_query(struct be_adapter *adapter, u16 *link_speed,
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_link_status *resp = embedded_payload(wrb);
+
 		if (link_speed) {
 			*link_speed = resp->link_speed ?
 				      le16_to_cpu(resp->link_speed) * 10 :
@@ -1670,6 +1682,7 @@ int be_cmd_get_reg_len(struct be_adapter *adapter, u32 *log_size)
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_get_fat *resp = embedded_payload(wrb);
+
 		if (log_size && resp->log_size)
 			*log_size = le32_to_cpu(resp->log_size) -
 					sizeof(u32);
@@ -1729,6 +1742,7 @@ int be_cmd_get_regs(struct be_adapter *adapter, u32 buf_len, void *buf)
 		status = be_mcc_notify_wait(adapter);
 		if (!status) {
 			struct be_cmd_resp_get_fat *resp = get_fat_cmd.va;
+
 			memcpy(buf + offset,
 			       resp->data_buffer,
 			       le32_to_cpu(resp->read_log_length));
@@ -2010,6 +2024,7 @@ int be_cmd_get_flow_control(struct be_adapter *adapter, u32 *tx_fc, u32 *rx_fc)
 	if (!status) {
 		struct be_cmd_resp_get_flow_control *resp =
 						embedded_payload(wrb);
+
 		*tx_fc = le16_to_cpu(resp->tx_flow_control);
 		*rx_fc = le16_to_cpu(resp->rx_flow_control);
 	}
@@ -2039,6 +2054,7 @@ int be_cmd_query_fw_cfg(struct be_adapter *adapter)
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_query_fw_cfg *resp = embedded_payload(wrb);
+
 		adapter->port_num = le32_to_cpu(resp->phys_port);
 		adapter->function_mode = le32_to_cpu(resp->function_mode);
 		adapter->function_caps = le32_to_cpu(resp->function_caps);
@@ -2187,6 +2203,7 @@ int be_cmd_get_beacon_state(struct be_adapter *adapter, u8 port_num, u32 *state)
 	if (!status) {
 		struct be_cmd_resp_get_beacon_state *resp =
 						embedded_payload(wrb);
+
 		*state = resp->beacon_state;
 	}
 
@@ -2628,6 +2645,7 @@ int be_cmd_ddr_dma_test(struct be_adapter *adapter, u64 pattern,
 
 	if (!status) {
 		struct be_cmd_resp_ddrdma_test *resp;
+
 		resp = cmd->va;
 		if ((memcmp(resp->rcv_buff, req->snd_buff, byte_cnt) != 0) ||
 				resp->snd_err) {
@@ -2703,6 +2721,7 @@ int be_cmd_get_phy_info(struct be_adapter *adapter)
 	if (!status) {
 		struct be_phy_info *resp_phy_info =
 				cmd.va + sizeof(struct be_cmd_req_hdr);
+
 		adapter->phy.phy_type = le16_to_cpu(resp_phy_info->phy_type);
 		adapter->phy.interface_type =
 			le16_to_cpu(resp_phy_info->interface_type);
@@ -2832,6 +2851,7 @@ int be_cmd_req_native_mode(struct be_adapter *adapter)
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_set_func_cap *resp = embedded_payload(wrb);
+
 		adapter->be3_native = le32_to_cpu(resp->cap_flags) &
 					CAPABILITY_BE3_NATIVE_ERX_API;
 		if (!adapter->be3_native)
@@ -2871,6 +2891,7 @@ int be_cmd_get_fn_privileges(struct be_adapter *adapter, u32 *privilege,
 	if (!status) {
 		struct be_cmd_resp_get_fn_privileges *resp =
 						embedded_payload(wrb);
+
 		*privilege = le32_to_cpu(resp->privilege_mask);
 
 		/* In UMC mode FW does not return right privileges.
@@ -3202,6 +3223,7 @@ int be_cmd_get_hsw_config(struct be_adapter *adapter, u16 *pvid,
 	if (!status) {
 		struct be_cmd_resp_get_hsw_config *resp =
 						embedded_payload(wrb);
+
 		be_dws_le_to_cpu(&resp->context, sizeof(resp->context));
 		vid = AMAP_GET_BITS(struct amap_get_hsw_resp_context,
 				    pvid, &resp->context);
@@ -3261,6 +3283,7 @@ int be_cmd_get_acpi_wol_cap(struct be_adapter *adapter)
 	status = be_mbox_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_acpi_wol_magic_config_v1 *resp;
+
 		resp = (struct be_cmd_resp_acpi_wol_magic_config_v1 *) cmd.va;
 
 		adapter->wol_cap = resp->wol_settings;
@@ -3297,6 +3320,7 @@ int be_cmd_set_fw_log_level(struct be_adapter *adapter, u32 level)
 			(extfat_cmd.va + sizeof(struct be_cmd_resp_hdr));
 	for (i = 0; i < le32_to_cpu(cfgs->num_modules); i++) {
 		u32 num_modes = le32_to_cpu(cfgs->module[i].num_modes);
+
 		for (j = 0; j < num_modes; j++) {
 			if (cfgs->module[i].trace_lvl[j].mode == MODE_UART)
 				cfgs->module[i].trace_lvl[j].dbg_lvl =
@@ -3333,6 +3357,7 @@ int be_cmd_get_fw_log_level(struct be_adapter *adapter)
 	if (!status) {
 		cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
 						sizeof(struct be_cmd_resp_hdr));
+
 		for (j = 0; j < le32_to_cpu(cfgs->module[0].num_modes); j++) {
 			if (cfgs->module[0].trace_lvl[j].mode == MODE_UART)
 				level = cfgs->module[0].trace_lvl[j].dbg_lvl;
@@ -3429,6 +3454,7 @@ int be_cmd_query_port_name(struct be_adapter *adapter, u8 *port_name)
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_get_port_name *resp = embedded_payload(wrb);
+
 		*port_name = resp->port_name[adapter->hba_port_num];
 	} else {
 		*port_name = adapter->hba_port_num + '0';
@@ -4052,6 +4078,7 @@ int be_cmd_get_active_profile(struct be_adapter *adapter, u16 *profile_id)
 	if (!status) {
 		struct be_cmd_resp_get_active_profile *resp =
 							embedded_payload(wrb);
+
 		*profile_id = le16_to_cpu(resp->active_profile_id);
 	}
 
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 32c61f3..eaf7003 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -1193,6 +1193,7 @@ static int be_set_rxfh(struct net_device *netdev, const u32 *indir,
 
 	if (indir) {
 		struct be_rx_obj *rxo;
+
 		for (i = 0; i < RSS_INDIR_TABLE_LEN; i++) {
 			j = indir[i];
 			rxo = &adapter->rx_obj[j];
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 96be231..fbab943 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -126,6 +126,7 @@ static const char * const ue_status_hi_desc[] = {
 static void be_queue_free(struct be_adapter *adapter, struct be_queue_info *q)
 {
 	struct be_dma_mem *mem = &q->dma_mem;
+
 	if (mem->va) {
 		dma_free_coherent(&adapter->pdev->dev, mem->size, mem->va,
 				  mem->dma);
@@ -187,6 +188,7 @@ static void be_intr_set(struct be_adapter *adapter, bool enable)
 static void be_rxq_notify(struct be_adapter *adapter, u16 qid, u16 posted)
 {
 	u32 val = 0;
+
 	val |= qid & DB_RQ_RING_ID_MASK;
 	val |= posted << DB_RQ_NUM_POSTED_SHIFT;
 
@@ -198,6 +200,7 @@ static void be_txq_notify(struct be_adapter *adapter, struct be_tx_obj *txo,
 			  u16 posted)
 {
 	u32 val = 0;
+
 	val |= txo->q.id & DB_TXULP_RING_ID_MASK;
 	val |= (posted & DB_TXULP_NUM_POSTED_MASK) << DB_TXULP_NUM_POSTED_SHIFT;
 
@@ -209,6 +212,7 @@ static void be_eq_notify(struct be_adapter *adapter, u16 qid,
 			 bool arm, bool clear_int, u16 num_popped)
 {
 	u32 val = 0;
+
 	val |= qid & DB_EQ_RING_ID_MASK;
 	val |= ((qid & DB_EQ_RING_ID_EXT_MASK) << DB_EQ_RING_ID_EXT_MASK_SHIFT);
 
@@ -227,6 +231,7 @@ static void be_eq_notify(struct be_adapter *adapter, u16 qid,
 void be_cq_notify(struct be_adapter *adapter, u16 qid, bool arm, u16 num_popped)
 {
 	u32 val = 0;
+
 	val |= qid & DB_CQ_RING_ID_MASK;
 	val |= ((qid & DB_CQ_RING_ID_EXT_MASK) <<
 			DB_CQ_RING_ID_EXT_MASK_SHIFT);
@@ -588,6 +593,7 @@ static struct rtnl_link_stats64 *be_get_stats64(struct net_device *netdev,
 
 	for_all_rx_queues(adapter, rxo, i) {
 		const struct be_rx_stats *rx_stats = rx_stats(rxo);
+
 		do {
 			start = u64_stats_fetch_begin_irq(&rx_stats->sync);
 			pkts = rx_stats(rxo)->rx_pkts;
@@ -602,6 +608,7 @@ static struct rtnl_link_stats64 *be_get_stats64(struct net_device *netdev,
 
 	for_all_tx_queues(adapter, txo, i) {
 		const struct be_tx_stats *tx_stats = tx_stats(txo);
+
 		do {
 			start = u64_stats_fetch_begin_irq(&tx_stats->sync);
 			pkts = tx_stats(txo)->tx_pkts;
@@ -807,6 +814,7 @@ static int make_tx_wrbs(struct be_adapter *adapter, struct be_queue_info *txq,
 
 	if (skb->len > skb->data_len) {
 		int len = skb_headlen(skb);
+
 		busaddr = dma_map_single(dev, skb->data, len, DMA_TO_DEVICE);
 		if (dma_mapping_error(dev, busaddr))
 			goto dma_err;
@@ -820,6 +828,7 @@ static int make_tx_wrbs(struct be_adapter *adapter, struct be_queue_info *txq,
 
 	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
 		const struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i];
+
 		busaddr = skb_frag_dma_map(dev, frag, 0,
 					   skb_frag_size(frag), DMA_TO_DEVICE);
 		if (dma_mapping_error(dev, busaddr))
diff --git a/drivers/net/ethernet/emulex/benet/be_roce.c b/drivers/net/ethernet/emulex/benet/be_roce.c
index ef4672d..1328664 100644
--- a/drivers/net/ethernet/emulex/benet/be_roce.c
+++ b/drivers/net/ethernet/emulex/benet/be_roce.c
@@ -174,6 +174,7 @@ int be_roce_register_driver(struct ocrdma_driver *drv)
 	ocrdma_drv = drv;
 	list_for_each_entry(dev, &be_adapter_list, entry) {
 		struct net_device *netdev;
+
 		_be_roce_dev_add(dev);
 		netdev = dev->netdev;
 		if (netif_running(netdev) && netif_oper_up(netdev))
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 04/12] be2net: add speed reporting for 20G-KR interface
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Vasundhara Volam <vasundhara.volam@emulex.com>

This patch adds speed reporting via ethtool for 20G KR2 interface on the
Skyhawk-R chip.

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.h    |    1 +
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    7 +++++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index b064c50..eb5085d 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1374,6 +1374,7 @@ enum {
 #define BE_SUPPORTED_SPEED_100MBPS	2
 #define BE_SUPPORTED_SPEED_1GBPS	4
 #define BE_SUPPORTED_SPEED_10GBPS	8
+#define BE_SUPPORTED_SPEED_20GBPS	0x10
 #define BE_SUPPORTED_SPEED_40GBPS	0x20
 
 #define BE_AN_EN			0x2
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 8297824..ec66f64 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -534,6 +534,13 @@ static u32 convert_to_et_setting(struct be_adapter *adapter, u32 if_speeds)
 		if (if_speeds & BE_SUPPORTED_SPEED_10GBPS)
 			val |= SUPPORTED_10000baseKX4_Full;
 		break;
+	case PHY_TYPE_KR2_20GB:
+		val |= SUPPORTED_Backplane;
+		if (if_speeds & BE_SUPPORTED_SPEED_10GBPS)
+			val |= SUPPORTED_10000baseKR_Full;
+		if (if_speeds & BE_SUPPORTED_SPEED_20GBPS)
+			val |= SUPPORTED_20000baseKR2_Full;
+		break;
 	case PHY_TYPE_KR_10GB:
 		val |= SUPPORTED_Backplane |
 				SUPPORTED_10000baseKR_Full;
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 02/12] be2net: fix sparse warnings in be_cmd_req_port_type{}
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1411121822-25842-1-git-send-email-sathya.perla@emulex.com>

From: Suresh Reddy <Suresh.Reddy@emulex.com>

This patch fixes a sprase warnings regarding endian declarations introduced
by the following commit:

fixes: e36edd9 ("be2net: add ethtool "-m" option support")

Signed-off-by: Suresh Reddy <Suresh.Reddy@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index e86a5ef..b064c50 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1005,8 +1005,8 @@ struct be_cmd_resp_link_status {
 /*    Identifies the type of port attached to NIC     */
 struct be_cmd_req_port_type {
 	struct be_cmd_req_hdr hdr;
-	u32 page_num;
-	u32 port;
+	__le32 page_num;
+	__le32 port;
 };
 
 enum {
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 00/12] be2net: patch set
From: Sathya Perla @ 2014-09-19 10:16 UTC (permalink / raw)
  To: netdev

Patches 1 and 2 fix sparse warnings (static declaration needed and endian
declaration needed) introduced by the earlier patch set.

Patches 3 and 4 add 20G/40G speed reporting via ethtool for the Skyhawk-R
chip.

Patches 5 to 12 fix various style issues and checkpatch warnings in the
driver such as:
	- removing unnecessary return statements in void routines
	- adding needed blank lines after a declaration block
	- deleting multiple blank lines
	- inserting a blank line after a function/struct definition
	- removing space after typecast
	- fixing multiple assignments on a single line
	- fixing alignment on a line wrap

Please consider applying this patch set to the net-next tree. Thanks!

Kalesh AP (10):
  be2net: fix a sparse warning in be_cmd_modify_eqd()
  be2net: add speed reporting for 40G/KR interface
  be2net: remove return statements for void functions
  be2net: add blank line after declarations
  be2net: remove multiple blank lines
  be2net: insert a blank line after function/struct//enum definitions
  be2net: remove unnecessary blank lines after an open brace
  be2net: remove space after typecasts
  be2net: remove multiple assignments on a single line
  be2net: fix alignment on line wrap

Suresh Reddy (1):
  be2net: fix sparse warnings in be_cmd_req_port_type{}

Vasundhara Volam (1):
  be2net: add speed reporting for 20G-KR interface

^ permalink raw reply

* Re: [PATCH 1/2 v2 nf-next] net: bridge: don't register netfilter call_iptables hooks by default
From: Florian Westphal @ 2014-09-19 10:13 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel, netdev
In-Reply-To: <20140918183956.GA3989@salvia>

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> We can keep those sysctls there in bridge.ko and deprecated them in
> favour of br_netlink. I think we can add some IFLA_BRPORT_NF for the
> setlink command to replace the existing global proc nf-call-thing.
> We'll have to enhance the 'bridge' tool in iproute2 to support this.

Not sure this is needed.  Patrick added support for a per-bridge
sysfs flag a while back.

> > Then, in two years or so, we would remove the autoload hook
> > in the packet procesing path.
> 
> Agreed. By that time, we can kill the bridge.ko <-> br_netfilter.ko
> dependency and the /proc call-nf-bridge stuff in favour of br_netlink.
> 
> Let me know if you still have any concern. Thanks.

Yes, well... I am afraid Patrick is right, we cannot remove it without
breaking some setups.

So, what to do?

I think the first step would be to go ahead and split the hook glue to
a separate module, and then add the autoprobing from the packet processing
path (only loading br_nf module when needed).

We could still add a deprecation warning later on (aka 'please modprobe
br_nf_core' instead).  But yes, Patricks probably right, waiting two
years doesn't make it more 'safe' than waiting 6 months...

What _might_ help is if there was an easy (and fast) way to tell wheter
iptables rules are loaded (right now it checks for active netfilter hooks).

Then, we could restrict the autoload to arp/ip(6)tables and not load
the br_nf glue module when nft is used instead.

That will allow removing autload with iptables removal.  Yes, I know
this is in a very distant future...

^ permalink raw reply

* Re: [PATCH net-next v4 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-19 10:08 UTC (permalink / raw)
  To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	David Miller
  Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <541BFEA4.9080702@redhat.com>

On 09/19/2014 12:00 PM, Nikolay Aleksandrov wrote:
> On 09/18/2014 11:53 PM, Mahesh Bandewar wrote:
>> Earlier change to use usable slave array for TLB mode had an additional
>> performance advantage. So extending the same logic to all other modes
>> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
>> Also consolidating this with the earlier TLB change.
>>
>> The main idea is to build the usable slaves array in the control path
>> and use that array for slave selection during xmit operation.
>>
>> Measured performance in a setup with a bond of 4x1G NICs with 200
>> instances of netperf for the modes involved (3ad, xor, tlb)
>> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>>
>> Mode        TPS-Before   TPS-After
>>
>> 802.3ad   : 468,694      493,101
>> TLB (lb=0): 392,583      392,965
>> XOR       : 475,696      484,517
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>> v1:
>>   (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
>>       the slave that need to be removed.
>>   (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>>       transition gracefully.
>>   (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
>>       failure.
>>   (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
>>       will populate the array even if these parameters are not used.
>>   (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>> v2:
>>   (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>>   (b) Slave link-events now refresh array for all these modes.
>>   (c) Moved free-array call from bond_close() to bond_uninit().
>> v3:
>>   (a) Fixed null pointer dereference.
>>   (b) Removed bond->lock lockdep dependency.
>> v4:
>>   (a) Made to changes to comply with Nikolay's locking changes
>>   (b) Added a work-queue to refresh slave-array when RTNL is not held
>>   (c) Array refresh happens ONLY with RTNL now.
>>   (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
>>
<<<<snip>>>>>
>> @@ -1963,6 +1972,10 @@ static void bond_miimon_commit(struct bonding *bond)
>>  				bond_alb_handle_link_change(bond, slave,
>>  							    BOND_LINK_UP);
>>  
>> +			if (BOND_MODE(bond) == BOND_MODE_XOR &&
>> +			    bond_update_slave_arr(bond, NULL))
>> +				pr_err("Failed to build slave-array for XOR mode.\n");
>> +
> miimon is also supported in the other hash using modes, it's used to look
> for link failure and speed/duplex changes. There's even a warning about it
> for 802.3ad/TLB/ALB modes:
> pr_warn("Warning: miimon must be specified, otherwise bonding will not
> detect link failure, speed and duplex which are essential for 802.3ad
> operation\n");
> pr_warn("Forcing miimon to 100msec\n");
> 
> bond_main.c: line 4026
> 
Actually nevermind this comment, their arrays will get rebuilt in their
respective link handling functions. I just thought we could somehow fold
these rebuilds but it seems impossible currently.

Nik

^ permalink raw reply

* Re: [PATCH net-next v4 1/2] bonding: display xmit_hash_policy for non-dynamic-tlb mode
From: Nikolay Aleksandrov @ 2014-09-19 10:01 UTC (permalink / raw)
  To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	David Miller
  Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <1411077194-20069-1-git-send-email-maheshb@google.com>

On 09/18/2014 11:53 PM, Mahesh Bandewar wrote:
> It's a trivial fix to display xmit_hash_policy for this new TLB mode
> since it uses transmit-hash-poilicy as part of bonding-master info
> (/proc/net/bonding/<bonding-interface).
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> v1
>  Rebase
> v2
>  Added bond_mode_uses_xmit_hash() inline function
> v3
>  Rebase
> v4
>  Rebase
> 
>  drivers/net/bonding/bond_procfs.c | 3 +--
>  drivers/net/bonding/bonding.h     | 7 +++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 

Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>

^ permalink raw reply

* Re: [PATCH net-next v4 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Nikolay Aleksandrov @ 2014-09-19 10:00 UTC (permalink / raw)
  To: Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
	David Miller
  Cc: netdev, Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <1411077211-20117-1-git-send-email-maheshb@google.com>

On 09/18/2014 11:53 PM, Mahesh Bandewar wrote:
> Earlier change to use usable slave array for TLB mode had an additional
> performance advantage. So extending the same logic to all other modes
> that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
> Also consolidating this with the earlier TLB change.
> 
> The main idea is to build the usable slaves array in the control path
> and use that array for slave selection during xmit operation.
> 
> Measured performance in a setup with a bond of 4x1G NICs with 200
> instances of netperf for the modes involved (3ad, xor, tlb)
> cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
> 
> Mode        TPS-Before   TPS-After
> 
> 802.3ad   : 468,694      493,101
> TLB (lb=0): 392,583      392,965
> XOR       : 475,696      484,517
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> v1:
>   (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
>       the slave that need to be removed.
>   (b) Freeing of array will assign NULL (to handle bond->down to bond->up
>       transition gracefully.
>   (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
>       failure.
>   (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
>       will populate the array even if these parameters are not used.
>   (e) 3AD: Should handle the ad_agg_selection_logic correctly.
> v2:
>   (a) Removed rcu_read_{un}lock() calls from array manipulation code.
>   (b) Slave link-events now refresh array for all these modes.
>   (c) Moved free-array call from bond_close() to bond_uninit().
> v3:
>   (a) Fixed null pointer dereference.
>   (b) Removed bond->lock lockdep dependency.
> v4:
>   (a) Made to changes to comply with Nikolay's locking changes
>   (b) Added a work-queue to refresh slave-array when RTNL is not held
>   (c) Array refresh happens ONLY with RTNL now.
>   (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
> 
Hello Mahesh,
This looks much better, I think we've ironed out most of the issues. A few
suggestions and one issue below.
First, I think you can fold the pr_err()s from the failing slave array
update into bond_update_slave_arr(), there's no error handling or rollback
so you can save a few lines and some complexity there and just check for
the mode and do the slave array update, if there's an error it can output
it itself. The rest is inlined below.


>  drivers/net/bonding/bond_3ad.c  |  88 +++++--------------
>  drivers/net/bonding/bond_alb.c  |  51 ++---------
>  drivers/net/bonding/bond_alb.h  |   8 --
>  drivers/net/bonding/bond_main.c | 189 ++++++++++++++++++++++++++++++++++++++--
>  drivers/net/bonding/bonding.h   |  10 +++
>  5 files changed, 218 insertions(+), 128 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 7e9e522fd476..4bf3756dcc11 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -1550,6 +1550,11 @@ static void ad_agg_selection_logic(struct aggregator *agg)
>  				__disable_port(port);
>  			}
>  		}
> +		/* RTNL may or may not be held but bond->mode_lock
> +		 * is held. It's not safe to update slave-arr here.
> +		 * Defer it to delayed-work.
> +		 */
I don't think the information about RTNL matters, the important point is
that mode_lock is held thus we can't update so we defer it. IMO You can
drop the RTNL part here and shorten it a bit :-)

> +		bond_slave_arr_work_rearm(bond);
>  	}
>  
>  	/* if the selected aggregator is of join individuals
> @@ -1688,6 +1693,11 @@ static void ad_enable_collecting_distributing(struct port *port)
>  			 port->actor_port_number,
>  			 port->aggregator->aggregator_identifier);
>  		__enable_port(port);
> +		/* RTNL is not be held and bond->mode_lock is held.
s/is not be/is not/

> +		 * It's not safe to update slave-arr here!
> +		 * Defer it to delayed-work.
> +		 */
> +		bond_slave_arr_work_rearm(port->slave->bond);
>  	}
>  }
>  
> @@ -1704,6 +1714,11 @@ static void ad_disable_collecting_distributing(struct port *port)
>  			 port->actor_port_number,
>  			 port->aggregator->aggregator_identifier);
>  		__disable_port(port);
> +		/* RTNL is not be held and bond->mode_lock is held.
Same here.

> +		 * It's not safe to update slave-arr here!
> +		 * Defer it to delayed-work.
> +		 */
> +		bond_slave_arr_work_rearm(port->slave->bond);
>  	}
>  }
>  
> @@ -2283,6 +2298,12 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
>  	port->sm_vars |= AD_PORT_BEGIN;
>  
>  	spin_unlock_bh(&slave->bond->mode_lock);
> +
> +	/* RTNL is held and mode_lock is released so it's safe
> +	 * to update slave_array here.
> +	 */
> +	if (bond_update_slave_arr(slave->bond, NULL))
> +		pr_err("Failed to build slave-array for 3ad mode.\n");
>  }
>  
>  /**
> @@ -2377,73 +2398,6 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
>  	return ret;
>  }
>  
> -int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
> -{
> -	struct bonding *bond = netdev_priv(dev);
> -	struct slave *slave, *first_ok_slave;
> -	struct aggregator *agg;
> -	struct ad_info ad_info;
> -	struct list_head *iter;
> -	int slaves_in_agg;
> -	int slave_agg_no;
> -	int agg_id;
> -
> -	if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
> -		netdev_dbg(dev, "__bond_3ad_get_active_agg_info failed\n");
> -		goto err_free;
> -	}
> -
> -	slaves_in_agg = ad_info.ports;
> -	agg_id = ad_info.aggregator_id;
> -
> -	if (slaves_in_agg == 0) {
> -		netdev_dbg(dev, "active aggregator is empty\n");
> -		goto err_free;
> -	}
> -
> -	slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
> -	first_ok_slave = NULL;
> -
> -	bond_for_each_slave_rcu(bond, slave, iter) {
> -		agg = SLAVE_AD_INFO(slave)->port.aggregator;
> -		if (!agg || agg->aggregator_identifier != agg_id)
> -			continue;
> -
> -		if (slave_agg_no >= 0) {
> -			if (!first_ok_slave && bond_slave_can_tx(slave))
> -				first_ok_slave = slave;
> -			slave_agg_no--;
> -			continue;
> -		}
> -
> -		if (bond_slave_can_tx(slave)) {
> -			bond_dev_queue_xmit(bond, skb, slave->dev);
> -			goto out;
> -		}
> -	}
> -
> -	if (slave_agg_no >= 0) {
> -		netdev_err(dev, "Couldn't find a slave to tx on for aggregator ID %d\n",
> -			   agg_id);
> -		goto err_free;
> -	}
> -
> -	/* we couldn't find any suitable slave after the agg_no, so use the
> -	 * first suitable found, if found.
> -	 */
> -	if (first_ok_slave)
> -		bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
> -	else
> -		goto err_free;
> -
> -out:
> -	return NETDEV_TX_OK;
> -err_free:
> -	/* no suitable interface, frame not sent */
> -	dev_kfree_skb_any(skb);
> -	goto out;
> -}
> -
>  int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
>  			 struct slave *slave)
>  {
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 615f3bebd019..d2eadab787c5 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -177,7 +177,6 @@ static int tlb_initialize(struct bonding *bond)
>  static void tlb_deinitialize(struct bonding *bond)
>  {
>  	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> -	struct tlb_up_slave *arr;
>  
>  	spin_lock_bh(&bond->mode_lock);
>  
> @@ -185,10 +184,6 @@ static void tlb_deinitialize(struct bonding *bond)
>  	bond_info->tx_hashtbl = NULL;
>  
>  	spin_unlock_bh(&bond->mode_lock);
> -
> -	arr = rtnl_dereference(bond_info->slave_arr);
> -	if (arr)
> -		kfree_rcu(arr, rcu);
>  }
>  
>  static long long compute_gap(struct slave *slave)
> @@ -1336,39 +1331,9 @@ out:
>  	return NETDEV_TX_OK;
>  }
>  
> -static int bond_tlb_update_slave_arr(struct bonding *bond,
> -				     struct slave *skipslave)
> -{
> -	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> -	struct slave *tx_slave;
> -	struct list_head *iter;
> -	struct tlb_up_slave *new_arr, *old_arr;
> -
> -	new_arr = kzalloc(offsetof(struct tlb_up_slave, arr[bond->slave_cnt]),
> -			  GFP_ATOMIC);
> -	if (!new_arr)
> -		return -ENOMEM;
> -
> -	bond_for_each_slave(bond, tx_slave, iter) {
> -		if (!bond_slave_can_tx(tx_slave))
> -			continue;
> -		if (skipslave == tx_slave)
> -			continue;
> -		new_arr->arr[new_arr->count++] = tx_slave;
> -	}
> -
> -	old_arr = rtnl_dereference(bond_info->slave_arr);
> -	rcu_assign_pointer(bond_info->slave_arr, new_arr);
> -	if (old_arr)
> -		kfree_rcu(old_arr, rcu);
> -
> -	return 0;
> -}
> -
>  int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>  {
>  	struct bonding *bond = netdev_priv(bond_dev);
> -	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>  	struct ethhdr *eth_data;
>  	struct slave *tx_slave = NULL;
>  	u32 hash_index;
> @@ -1389,12 +1354,14 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>  							      hash_index & 0xFF,
>  							      skb->len);
>  			} else {
> -				struct tlb_up_slave *slaves;
> +				struct bond_up_slave *slaves;
> +				unsigned int count;
>  
> -				slaves = rcu_dereference(bond_info->slave_arr);
> -				if (slaves && slaves->count)
> +				slaves = rcu_dereference(bond->slave_arr);
> +				count = slaves ? ACCESS_ONCE(slaves->count) : 0;
> +				if (likely(count))
>  					tx_slave = slaves->arr[hash_index %
> -							       slaves->count];
> +							       count];
>  			}
>  			break;
>  		}
> @@ -1641,10 +1608,6 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
>  		rlb_clear_slave(bond, slave);
>  	}
>  
> -	if (bond_is_nondyn_tlb(bond))
> -		if (bond_tlb_update_slave_arr(bond, slave))
> -			pr_err("Failed to build slave-array for TLB mode.\n");
> -
>  }
>  
>  void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char link)
> @@ -1669,7 +1632,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
>  	}
>  
>  	if (bond_is_nondyn_tlb(bond)) {
> -		if (bond_tlb_update_slave_arr(bond, NULL))
> +		if (bond_update_slave_arr(bond, NULL))
>  			pr_err("Failed to build slave-array for TLB mode.\n");
>  	}
>  }
> diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
> index 3c6a7ff974d7..1ad473b4ade5 100644
> --- a/drivers/net/bonding/bond_alb.h
> +++ b/drivers/net/bonding/bond_alb.h
> @@ -139,19 +139,11 @@ struct tlb_slave_info {
>  			 */
>  };
>  
> -struct tlb_up_slave {
> -	unsigned int	count;
> -	struct rcu_head rcu;
> -	struct slave	*arr[0];
> -};
> -
>  struct alb_bond_info {
>  	struct tlb_client_info	*tx_hashtbl; /* Dynamically allocated */
>  	u32			unbalanced_load;
>  	int			tx_rebalance_counter;
>  	int			lp_counter;
> -	/* -------- non-dynamic tlb mode only ---------*/
> -	struct tlb_up_slave __rcu *slave_arr;	  /* Up slaves */
>  	/* -------- rlb parameters -------- */
>  	int rlb_enabled;
>  	struct rlb_client_info	*rx_hashtbl;	/* Receive hash table */
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 5e7987bba583..e87b802d8813 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -208,6 +208,7 @@ static int lacp_fast;
>  
>  static int bond_init(struct net_device *bond_dev);
>  static void bond_uninit(struct net_device *bond_dev);
> +static void bond_slave_arr_handler(struct work_struct *work);
>  
>  /*---------------------------- General routines -----------------------------*/
>  
> @@ -1547,6 +1548,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>  		unblock_netpoll_tx();
>  	}
>  
> +	if (bond_mode_uses_xmit_hash(bond) &&
> +	    bond_update_slave_arr(bond, NULL))
> +		pr_err("Failed to build slave-array.\n");
> +
>  	netdev_info(bond_dev, "Enslaving %s as %s interface with %s link\n",
>  		    slave_dev->name,
>  		    bond_is_active_slave(new_slave) ? "an active" : "a backup",
> @@ -1661,6 +1666,10 @@ static int __bond_release_one(struct net_device *bond_dev,
>  	if (BOND_MODE(bond) == BOND_MODE_8023AD)
>  		bond_3ad_unbind_slave(slave);
>  
> +	if (bond_mode_uses_xmit_hash(bond) &&
> +	    bond_update_slave_arr(bond, slave))
> +		pr_err("Failed to build slave-array.\n");
> +
>  	netdev_info(bond_dev, "Releasing %s interface %s\n",
>  		    bond_is_active_slave(slave) ? "active" : "backup",
>  		    slave_dev->name);
> @@ -1963,6 +1972,10 @@ static void bond_miimon_commit(struct bonding *bond)
>  				bond_alb_handle_link_change(bond, slave,
>  							    BOND_LINK_UP);
>  
> +			if (BOND_MODE(bond) == BOND_MODE_XOR &&
> +			    bond_update_slave_arr(bond, NULL))
> +				pr_err("Failed to build slave-array for XOR mode.\n");
> +
miimon is also supported in the other hash using modes, it's used to look
for link failure and speed/duplex changes. There's even a warning about it
for 802.3ad/TLB/ALB modes:
pr_warn("Warning: miimon must be specified, otherwise bonding will not
detect link failure, speed and duplex which are essential for 802.3ad
operation\n");
pr_warn("Forcing miimon to 100msec\n");

bond_main.c: line 4026

>  			if (!bond->curr_active_slave || slave == primary)
>  				goto do_failover;
>  
> @@ -1990,6 +2003,10 @@ static void bond_miimon_commit(struct bonding *bond)
>  				bond_alb_handle_link_change(bond, slave,
>  							    BOND_LINK_DOWN);
>  
> +			if (BOND_MODE(bond) == BOND_MODE_XOR &&
> +			    bond_update_slave_arr(bond, NULL))
> +				pr_err("Failed to build slave-array for XOR mode.\n");
> +
Same here.

>  			if (slave == rcu_access_pointer(bond->curr_active_slave))
>  				goto do_failover;
>  
> @@ -2446,6 +2463,9 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
>  
>  		if (slave_state_changed) {
>  			bond_slave_state_change(bond);
> +			if (BOND_MODE(bond) == BOND_MODE_XOR &&
> +			    bond_update_slave_arr(bond, NULL))
> +				pr_err("Failed to build slave-array for XOR mode.\n");
>  		} else if (do_failover) {
>  			block_netpoll_tx();
>  			bond_select_active_slave(bond);
> @@ -2822,8 +2842,23 @@ static int bond_slave_netdev_event(unsigned long event,
>  			if (old_duplex != slave->duplex)
>  				bond_3ad_adapter_duplex_changed(slave);
>  		}
> +		/* Refresh slave-array if applicable!
> +		 * If the setuo does not use miimon or arpmon (mode-specific!),
s/setuo/setup/

> +		 * then these event will not cause the slave-array to be
s/event/events/ ?

> +		 * refreshed. This will cause xmit to use a slave that is not
> +		 * usable. Avoid such situation by refeshing the array at these
> +		 * events. If these (miimon/arpmon) parameters are configured
> +		 * then array gets refreshed twice and that should be fine!
> +		 */
> +		if (bond_mode_uses_xmit_hash(bond) &&
> +		    bond_update_slave_arr(bond, NULL))
> +			pr_err("Failed to build slave-array.\n");
>  		break;
>  	case NETDEV_DOWN:
> +		/* Refresh slave-array if applicable! */
Please drop this comment, it doesn't bring any new information from the if().

> +		if (bond_mode_uses_xmit_hash(bond) &&
> +		    bond_update_slave_arr(bond, NULL))
> +			pr_err("Failed to build slave-array.\n");
>  		break;
>  	case NETDEV_CHANGEMTU:
>  		/* TODO: Should slaves be allowed to
> @@ -3003,6 +3038,7 @@ static void bond_work_init_all(struct bonding *bond)
>  	else
>  		INIT_DELAYED_WORK(&bond->arp_work, bond_loadbalance_arp_mon);
>  	INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
> +	INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler);
>  }
>  
>  static void bond_work_cancel_all(struct bonding *bond)
> @@ -3012,6 +3048,7 @@ static void bond_work_cancel_all(struct bonding *bond)
>  	cancel_delayed_work_sync(&bond->alb_work);
>  	cancel_delayed_work_sync(&bond->ad_work);
>  	cancel_delayed_work_sync(&bond->mcast_work);
> +	cancel_delayed_work_sync(&bond->slave_arr_work);
>  }
>  
>  static int bond_open(struct net_device *bond_dev)
> @@ -3061,6 +3098,10 @@ static int bond_open(struct net_device *bond_dev)
>  		bond_3ad_initiate_agg_selection(bond, 1);
>  	}
>  
> +	if (bond_mode_uses_xmit_hash(bond) &&
> +	    bond_update_slave_arr(bond, NULL))
> +		pr_err("Failed to build slave-array.\n");
> +
>  	return 0;
>  }
>  
> @@ -3555,15 +3596,139 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
>  	return NETDEV_TX_OK;
>  }
>  
> -/* In bond_xmit_xor() , we determine the output device by using a pre-
> - * determined xmit_hash_policy(), If the selected device is not enabled,
> - * find the next active slave.
> +/* The caller is holding bond->mode_lock and may or may not be
> + * holding RTNL.
>   */
I'd say change this comment to note that this should be used when it's not
appropriate to update the slave array right away F.e. when sleeping is not
an option or when RTNL isn't held. Currently it's only the mode_lock case,
but that may change and a more general comment would be more helpful.

> -static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
> +void bond_slave_arr_work_rearm(struct bonding *bond)
>  {
> -	struct bonding *bond = netdev_priv(bond_dev);
> +	queue_delayed_work(bond->wq, &bond->slave_arr_work, 1);
> +}
>  
> -	bond_xmit_slave_id(bond, skb, bond_xmit_hash(bond, skb) % bond->slave_cnt);
> +/* Slave array work handler. Holds only RTNL */
> +static void bond_slave_arr_handler(struct work_struct *work)
> +{
> +	struct bonding *bond = container_of(work, struct bonding,
> +					    slave_arr_work.work);
> +	int ret;
> +
> +	if (!rtnl_trylock())
> +		goto err;
> +
> +	ret = bond_update_slave_arr(bond, NULL);
> +	rtnl_unlock();
> +	if (ret) {
> +		pr_warn_ratelimited("Failed to update slave array from WT\n");
> +		goto err;
> +	}
> +	return;
> +
> +err:
> +	bond_slave_arr_work_rearm(bond);
> +}
> +
> +/* Build the usable slaves array in control path for modes that use xmit-hash
> + * to determine the slave interface -
> + * (a) BOND_MODE_8023AD
> + * (b) BOND_MODE_XOR
> + * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
> + *
> + * The caller is expected to hold RTNL only and NO other lock!
> + */
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
> +{
> +	struct slave *slave;
> +	struct list_head *iter;
> +	struct bond_up_slave *new_arr, *old_arr;
> +	int slaves_in_agg;
> +	int agg_id = 0;
> +	int ret = 0;
> +
> +#ifdef CONFIG_LOCKDEP
> +	WARN_ON(lockdep_is_held(&bond->mode_lock));
> +#endif
> +
> +	new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
> +			  GFP_KERNEL);
> +	if (!new_arr) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> +		struct ad_info ad_info;
> +
> +		if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
> +			pr_debug("bond_3ad_get_active_agg_info failed\n");
> +			kfree_rcu(new_arr, rcu);
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		slaves_in_agg = ad_info.ports;
> +		agg_id = ad_info.aggregator_id;
> +	}
> +	bond_for_each_slave(bond, slave, iter) {
> +		if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> +			struct aggregator *agg;
> +
> +			agg = SLAVE_AD_INFO(slave)->port.aggregator;
> +			if (!agg || agg->aggregator_identifier != agg_id)
> +				continue;
> +		}
> +		if (!bond_slave_can_tx(slave))
> +			continue;
> +		if (skipslave == slave)
> +			continue;
> +		new_arr->arr[new_arr->count++] = slave;
> +	}
> +
> +	old_arr = rtnl_dereference(bond->slave_arr);
> +	rcu_assign_pointer(bond->slave_arr, new_arr);
> +	if (old_arr)
> +		kfree_rcu(old_arr, rcu);
> +out:
> +	if (ret != 0 && skipslave) {
> +		int idx;
> +
> +		/* Rare situation where caller has asked to skip a specific
> +		 * slave but allocation failed (most likely!). BTW this is
> +		 * only possible when the call is initiated from
> +		 * __bond_release_one(). In this sitation; overwrite the
s/sitation/situation/

> +		 * skipslave entry in the array with the last entry from the
> +		 * array to avoid a situation where the xmit path may choose
> +		 * this to-be-skipped slave to send a packet out.
> +		 */
> +		old_arr = rtnl_dereference(bond->slave_arr);
> +		for (idx = 0; idx < old_arr->count; idx++) {
> +			if (skipslave == old_arr->arr[idx]) {
> +				old_arr->arr[idx] =
> +				    old_arr->arr[old_arr->count-1];
> +				old_arr->count--;
> +				break;
> +			}
> +		}
> +	}
> +	return ret;
> +}
> +
> +/* Use this Xmit function for 3AD as well as XOR modes. The current
> + * usable slave array is formed in the control path. The xmit function
> + * just calculates hash and sends the packet out.
> + */
> +int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct bonding *bond = netdev_priv(dev);
> +	struct slave *slave;
> +	struct bond_up_slave *slaves;
> +	unsigned int count;
> +
> +	slaves = rcu_dereference(bond->slave_arr);
> +	count = slaves ? ACCESS_ONCE(slaves->count) : 0;
> +	if (likely(count)) {
> +		slave = slaves->arr[bond_xmit_hash(bond, skb) % count];
> +		bond_dev_queue_xmit(bond, skb, slave->dev);
> +	} else {
> +		dev_kfree_skb_any(skb);
> +		atomic_long_inc(&dev->tx_dropped);
> +	}
>  
>  	return NETDEV_TX_OK;
>  }
> @@ -3660,12 +3825,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
>  		return bond_xmit_roundrobin(skb, dev);
>  	case BOND_MODE_ACTIVEBACKUP:
>  		return bond_xmit_activebackup(skb, dev);
> +	case BOND_MODE_8023AD:
>  	case BOND_MODE_XOR:
> -		return bond_xmit_xor(skb, dev);
> +		return bond_3ad_xor_xmit(skb, dev);
>  	case BOND_MODE_BROADCAST:
>  		return bond_xmit_broadcast(skb, dev);
> -	case BOND_MODE_8023AD:
> -		return bond_3ad_xmit_xor(skb, dev);
>  	case BOND_MODE_ALB:
>  		return bond_alb_xmit(skb, dev);
>  	case BOND_MODE_TLB:
> @@ -3839,6 +4003,7 @@ static void bond_uninit(struct net_device *bond_dev)
>  	struct bonding *bond = netdev_priv(bond_dev);
>  	struct list_head *iter;
>  	struct slave *slave;
> +	struct bond_up_slave *arr;
>  
>  	bond_netpoll_cleanup(bond_dev);
>  
> @@ -3847,6 +4012,12 @@ static void bond_uninit(struct net_device *bond_dev)
>  		__bond_release_one(bond_dev, slave->dev, true);
>  	netdev_info(bond_dev, "Released all slaves\n");
>  
> +	arr = rtnl_dereference(bond->slave_arr);
> +	if (arr) {
> +		kfree_rcu(arr, rcu);
> +		RCU_INIT_POINTER(bond->slave_arr, NULL);
> +	}
> +
>  	list_del(&bond->bond_list);
>  
>  	bond_debug_unregister(bond);
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 98dc0d7ad731..4635b175256a 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -177,6 +177,12 @@ struct slave {
>  	struct kobject kobj;
>  };
>  
> +struct bond_up_slave {
> +	unsigned int	count;
> +	struct rcu_head rcu;
> +	struct slave	*arr[0];
> +};
> +
>  /*
>   * Link pseudo-state only used internally by monitors
>   */
> @@ -191,6 +197,7 @@ struct bonding {
>  	struct   slave __rcu *curr_active_slave;
>  	struct   slave __rcu *current_arp_slave;
>  	struct   slave __rcu *primary_slave;
> +	struct   bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
>  	bool     force_primary;
>  	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
>  	int     (*recv_probe)(const struct sk_buff *, struct bonding *,
> @@ -220,6 +227,7 @@ struct bonding {
>  	struct   delayed_work alb_work;
>  	struct   delayed_work ad_work;
>  	struct   delayed_work mcast_work;
> +	struct   delayed_work slave_arr_work;
>  #ifdef CONFIG_DEBUG_FS
>  	/* debugging support via debugfs */
>  	struct	 dentry *debug_dir;
> @@ -531,6 +539,8 @@ const char *bond_slave_link_status(s8 link);
>  struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
>  					      struct net_device *end_dev,
>  					      int level);
> +int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
> +void bond_slave_arr_work_rearm(struct bonding *bond);
>  
>  #ifdef CONFIG_PROC_FS
>  void bond_create_proc_entry(struct bonding *bond);
> 

^ permalink raw reply

* RE: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected
From: Kweh, Hock Leong @ 2014-09-19  9:52 UTC (permalink / raw)
  To: Giuseppe CAVALLARO, David S. Miller, rayagond@vayavyalabs.com
  Cc: Vince Bridgers, Chen-Yu Tsai, netdev@vger.kernel.org, LKML,
	Ong, Boon Leong, Tobias Klausmann
In-Reply-To: <541AF116.9010004@st.com>

> -----Original Message-----
> From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com]
> Sent: Thursday, September 18, 2014 10:50 PM
> On 9/18/2014 2:34 PM, Kweh Hock Leong wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> 
> Hmm I am not sure this is the right fix. The driver has to fail if the main clock is
> not found. Indeed dev_warn has to be changed in dev_err.
> 
> Take a look at Documentation/networking/stmmac.txt but I will post some
> patch to improve the documentation adding further detail for clocks too.
> 
> The the logic behind the code is that the CSR clock will be set at runtime if in
> case of priv->plat->clk_csr ==0 or it will be forced to a fixed value if passed
> from the platform instead of.
> IIRC This was required on some platforms time ago.
> For sure the driver is designed to fail in case of no main clock is found.
> 
> Peppe

Hi Peppe,

I understand your point from the code below (at file stmmac_main.c line 2784):

/* If a specific clk_csr value is passed from the platform
  * this means that the CSR Clock Range selection cannot be
  * changed at run-time and it is fixed. Viceversa the driver'll try to
  * set the MDC clock dynamically according to the csr actual
  * clock input.
  */
if (!priv->plat->clk_csr)
        stmmac_clk_csr_set(priv);
else
        priv->clk_csr = priv->plat->clk_csr;


I did search through the whole stmmac_main.c file and found that only stmmac_clk_csr_set()
function is leveraging the priv->stmmac_clk params for it calculation. By the logic point of
view, I do not need priv->stmmac_clk when I got priv->plat->clk_csr. With this thinking,
I propose this fix as when the probe get priv->plat->clk_csr, it shouldn't fail if priv->stmmac_clk
has the error value.


Regards,
Wilson

^ permalink raw reply

* Re: [PATCH RFC ipsec-next] xfrm: Add sysctl option to enforce inbound policies for transport mode
From: Steffen Klassert @ 2014-09-19  9:24 UTC (permalink / raw)
  To: Tobias Brunner; +Cc: davem, netdev, Herbert Xu
In-Reply-To: <541815C3.7080509@strongswan.org>

Ccing Herbert Xu.

On Tue, Sep 16, 2014 at 12:49:39PM +0200, Tobias Brunner wrote:
> Currently inbound policies for transport mode SAs are not enforced.
> If no policy is found or if the templates don't match this is not
> considered an error for transport mode SAs.
> 

The strict inbound policy enforcement was implemented by Herbert.
The commit predates our git history but can be found in the history
tree:

git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

It was the following commit:

commit 8fe7ee2ba983fd89b2555dce5930ffd0f7f6c361
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Thu Oct 23 14:57:11 2003 -0700

    [IPSEC]: Strengthen policy checks.

Maybe Herbert remembers why this was done only for tunnel mode.

If I read section 5.2.1 of RFC 2401 correct, the inbound policy
must be enforced regardless of the mode.

> The new sysctl option (net.core.xfrm_enforce_policies_transport_mode)
> allows enforcing the inbound policies also for transport mode SAs.
> By default this option remains disabled.

I'd not like to have a sysctl for this. I consider it as a bug
if an installed policy can not be enforced, and we don't fix bugs
with sysctls :).

> 
> Signed-off-by: Tobias Brunner <tobias@strongswan.org>
> ---
> Consider a transport mode SA between two peers over which only TCP and
> ICMP traffic should be allowed.  Because two protocols are involved the
> selector on the SA itself (xfrm_usersa_info.sel) can't be set.  Instead
> one either has to negotiate separate SAs for each protocol (with a
> selector on each) or negotiate one SA and install two policies that
> use it.  The problem with the latter is that the peer does not have to
> adhere to the negotiated traffic selectors, it is basically free to
> send anything over the SA e.g. UDP packets.  So if no selectors are
> installed on the SA itself, the current implementation would accept
> such packets, even though the installed policies don't allow UDP
> traffic (some might consider this a security issue - at the very least
> it is not very intuitive, especially because the behavior is different
> for tunnel mode SAs).
> By enabling the new sysctl option the UDP packets would get dropped,
> exactly as they would if the SA were negotiated in tunnel mode.
> 
> The behavior for optional transport mode templates also changes when
> the option is enabled.  Basically, the special treatment of transport
> mode SAs is disabled and the behavior is like it is for other modes.
> I tested this with IPComp/ESP and strongSwan where the optional IPComp
> transform is installed in transport or tunnel mode depending on the
> negotiated mode of the SA (the ESP SA is always installed in transport
> mode in this combination).  But I'm not sure if there are other uses
> for optional transport mode transforms that might rely on the current
> behavior (i.e. why are optional templates treated differently depending
> on their mode in the first place?).

The code arround xfrm_policy_ok() looks a bit obscure, I'm not sure if
all combinations of required and optional templates are handled correct.

> 
> With this patch the default behavior remains as it is, but I wondered
> why it is like that and if we could perhaps change it so that policies
> are enforced by default, thus making such setups more secure out of
> the box.  Or if we could even change the whole inbound processing so
> that transport mode SAs are treated exactly like their counterparts
> in other modes, without an option to change it.
> 
>  Documentation/networking/xfrm_sysctl.txt |  4 ++++
>  include/net/netns/xfrm.h                 |  1 +
>  net/xfrm/xfrm_policy.c                   | 24 +++++++++++++++---------
>  net/xfrm/xfrm_sysctl.c                   |  8 ++++++++
>  4 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/networking/xfrm_sysctl.txt b/Documentation/networking/xfrm_sysctl.txt
> index 5bbd167..2fbe539 100644
> --- a/Documentation/networking/xfrm_sysctl.txt
> +++ b/Documentation/networking/xfrm_sysctl.txt
> @@ -2,3 +2,7 @@
>   xfrm_acq_expires - INTEGER
>  	default 30 - hard timeout in seconds for acquire requests
> +
> +xfrm_enforce_policies_transport_mode - BOOLEAN
> +	default 0 (disabled) - whether to enforce inbound policies for transport
> +	mode SAs
> \ No newline at end of file
> diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
> index 9da7982..e045ecc 100644
> --- a/include/net/netns/xfrm.h
> +++ b/include/net/netns/xfrm.h
> @@ -64,6 +64,7 @@ struct netns_xfrm {
>  	u32			sysctl_aevent_rseqth;
>  	int			sysctl_larval_drop;
>  	u32			sysctl_acq_expires;
> +	int			sysctl_enforce_policies_transport_mode;
>  #ifdef CONFIG_SYSCTL
>  	struct ctl_table_header	*sysctl_hdr;
>  #endif
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 55bcb86..5296f6b 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -2355,20 +2355,22 @@ xfrm_state_ok(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x,
>   * Otherwise "-2 - errored_index" is returned.
>   */
>  static inline int
> -xfrm_policy_ok(const struct xfrm_tmpl *tmpl, const struct sec_path *sp, int start,
> -	       unsigned short family)
> +xfrm_policy_ok(const struct net *net, const struct xfrm_tmpl *tmpl,
> +	       const struct sec_path *sp, int start, unsigned short family)
>  {
>  	int idx = start;
>   	if (tmpl->optional) {
> -		if (tmpl->mode == XFRM_MODE_TRANSPORT)
> +		if (tmpl->mode == XFRM_MODE_TRANSPORT &&
> +		    !net->xfrm.sysctl_enforce_policies_transport_mode)
>  			return start;
>  	} else
>  		start = -1;
>  	for (; idx < sp->len; idx++) {
>  		if (xfrm_state_ok(tmpl, sp->xvec[idx], family))
>  			return ++idx;
> -		if (sp->xvec[idx]->props.mode != XFRM_MODE_TRANSPORT) {
> +		if (sp->xvec[idx]->props.mode != XFRM_MODE_TRANSPORT ||
> +		    net->xfrm.sysctl_enforce_policies_transport_mode) {
>  			if (start == -1)
>  				start = -2-idx;
>  			break;
> @@ -2393,10 +2395,13 @@ int __xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
>  }
>  EXPORT_SYMBOL(__xfrm_decode_session);
>  -static inline int secpath_has_nontransport(const struct sec_path *sp, int k, int *idxp)
> +static inline int secpath_has_nontransport(const struct net *net,
> +					   const struct sec_path *sp,
> +					   int k, int *idxp)
>  {
>  	for (; k < sp->len; k++) {
> -		if (sp->xvec[k]->props.mode != XFRM_MODE_TRANSPORT) {
> +		if (sp->xvec[k]->props.mode != XFRM_MODE_TRANSPORT ||
> +		    net->xfrm.sysctl_enforce_policies_transport_mode) {
>  			*idxp = k;
>  			return 1;
>  		}
> @@ -2469,7 +2474,8 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
>  	}
>   	if (!pol) {
> -		if (skb->sp && secpath_has_nontransport(skb->sp, 0, &xerr_idx)) {
> +		if (skb->sp &&
> +		    secpath_has_nontransport(net, skb->sp, 0, &xerr_idx)) {
>  			xfrm_secpath_reject(xerr_idx, skb, &fl);
>  			XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
>  			return 0;
> @@ -2535,7 +2541,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
>  		 * are implied between each two transformations.
>  		 */
>  		for (i = xfrm_nr-1, k = 0; i >= 0; i--) {
> -			k = xfrm_policy_ok(tpp[i], sp, k, family);
> +			k = xfrm_policy_ok(net, tpp[i], sp, k, family);
>  			if (k < 0) {
>  				if (k < -1)
>  					/* "-2 - errored_index" returned */
> @@ -2545,7 +2551,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
>  			}
>  		}
>  -		if (secpath_has_nontransport(sp, k, &xerr_idx)) {
> +		if (secpath_has_nontransport(net, sp, k, &xerr_idx)) {
>  			XFRM_INC_STATS(net, LINUX_MIB_XFRMINTMPLMISMATCH);
>  			goto reject;
>  		}
> diff --git a/net/xfrm/xfrm_sysctl.c b/net/xfrm/xfrm_sysctl.c
> index 05a6e3d..17671af 100644
> --- a/net/xfrm/xfrm_sysctl.c
> +++ b/net/xfrm/xfrm_sysctl.c
> @@ -9,6 +9,7 @@ static void __net_init __xfrm_sysctl_init(struct net *net)
>  	net->xfrm.sysctl_aevent_rseqth = XFRM_AE_SEQT_SIZE;
>  	net->xfrm.sysctl_larval_drop = 1;
>  	net->xfrm.sysctl_acq_expires = 30;
> +	net->xfrm.sysctl_enforce_policies_transport_mode = 0;
>  }
>   #ifdef CONFIG_SYSCTL
> @@ -37,6 +38,12 @@ static struct ctl_table xfrm_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec
>  	},
> +	{
> +		.procname	= "xfrm_enforce_policies_transport_mode",
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec
> +	},
>  	{}
>  };
>  @@ -53,6 +60,7 @@ int __net_init xfrm_sysctl_init(struct net *net)
>  	table[1].data = &net->xfrm.sysctl_aevent_rseqth;
>  	table[2].data = &net->xfrm.sysctl_larval_drop;
>  	table[3].data = &net->xfrm.sysctl_acq_expires;
> +	table[4].data = &net->xfrm.sysctl_enforce_policies_transport_mode;
>   	/* Don't export sysctls to unprivileged users */
>  	if (net->user_ns != &init_user_ns)
> -- 
> 1.9.1

^ 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