Netdev List
 help / color / mirror / Atom feed
* [net-next v2 03/15] igb: Added rcu_lock to avoid race
From: Jeff Kirsher @ 2013-08-22  9:49 UTC (permalink / raw)
  To: davem
  Cc: Akeem G Abodunrin, netdev, gospo, sassmann, Eric Dumazet,
	Jeff Kirsher
In-Reply-To: <1377164991-27988-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>

This patch adds rcu_lock to avoid possible race condition with igb_update_stats
function accessing the rings in free_ q_vector.

CC: Eric Dumazet <edumazet@google.com>
Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 090ee56..8e7c1b8 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1013,7 +1013,7 @@ static void igb_free_q_vector(struct igb_adapter *adapter, int v_idx)
 	adapter->q_vector[v_idx] = NULL;
 	netif_napi_del(&q_vector->napi);
 
-	/* ixgbe_get_stats64() might access the rings on this vector,
+	/* igb_get_stats64() might access the rings on this vector,
 	 * we must wait a grace period before freeing it.
 	 */
 	kfree_rcu(q_vector, rcu);
@@ -4859,6 +4859,8 @@ void igb_update_stats(struct igb_adapter *adapter,
 
 	bytes = 0;
 	packets = 0;
+
+	rcu_read_lock();
 	for (i = 0; i < adapter->num_rx_queues; i++) {
 		u32 rqdpc = rd32(E1000_RQDPC(i));
 		struct igb_ring *ring = adapter->rx_ring[i];
@@ -4894,6 +4896,7 @@ void igb_update_stats(struct igb_adapter *adapter,
 	}
 	net_stats->tx_bytes = bytes;
 	net_stats->tx_packets = packets;
+	rcu_read_unlock();
 
 	/* read stats registers */
 	adapter->stats.crcerrs += rd32(E1000_CRCERRS);
-- 
1.8.3.1

^ permalink raw reply related

* [net-next v2 04/15] igb: don't allow SR-IOV without MSI-X
From: Jeff Kirsher @ 2013-08-22  9:49 UTC (permalink / raw)
  To: davem; +Cc: Mitch A Williams, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1377164991-27988-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Mitch A Williams <mitch.a.williams@intel.com>

MSI-X interrupts are required for SR-IOV operation. Check to make sure
they're enabled before allowing the user to turn on VFs.

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8e7c1b8..94295a0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2436,6 +2436,11 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
 	int err = 0;
 	int i;
 
+	if (!adapter->msix_entries) {
+		err = -EPERM;
+		goto out;
+	}
+
 	if (!num_vfs)
 		goto out;
 	else if (old_vfs && old_vfs == num_vfs)
-- 
1.8.3.1

^ permalink raw reply related

* [net-next v2 02/15] igb: Read register for latch_on without return value
From: Jeff Kirsher @ 2013-08-22  9:49 UTC (permalink / raw)
  To: davem; +Cc: Akeem G Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1377164991-27988-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>

This patch changes register read to "just-read" without returning a value
for hardware to accurately latch the register value.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c |  9 ++++-----
 drivers/net/ethernet/intel/igb/igb_main.c    | 14 ++++----------
 drivers/net/ethernet/intel/igb/igb_ptp.c     |  8 ++++----
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index f21a91a..9057d10 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -1320,7 +1320,7 @@ void igb_shutdown_serdes_link_82575(struct e1000_hw *hw)
  **/
 static s32 igb_reset_hw_82575(struct e1000_hw *hw)
 {
-	u32 ctrl, icr;
+	u32 ctrl;
 	s32 ret_val;
 
 	/* Prevent the PCI-E bus from sticking if there is no TLP connection
@@ -1365,7 +1365,7 @@ static s32 igb_reset_hw_82575(struct e1000_hw *hw)
 
 	/* Clear any pending interrupt events. */
 	wr32(E1000_IMC, 0xffffffff);
-	icr = rd32(E1000_ICR);
+	rd32(E1000_ICR);
 
 	/* Install any alternate MAC address into RAR0 */
 	ret_val = igb_check_alt_mac_addr(hw);
@@ -2103,10 +2103,9 @@ static s32 igb_reset_hw_82580(struct e1000_hw *hw)
 	s32 ret_val = 0;
 	/* BH SW mailbox bit in SW_FW_SYNC */
 	u16 swmbsw_mask = E1000_SW_SYNCH_MB;
-	u32 ctrl, icr;
+	u32 ctrl;
 	bool global_device_reset = hw->dev_spec._82575.global_device_reset;
 
-
 	hw->dev_spec._82575.global_device_reset = false;
 
 	/* due to hw errata, global device reset doesn't always
@@ -2165,7 +2164,7 @@ static s32 igb_reset_hw_82580(struct e1000_hw *hw)
 
 	/* Clear any pending interrupt events. */
 	wr32(E1000_IMC, 0xffffffff);
-	icr = rd32(E1000_ICR);
+	rd32(E1000_ICR);
 
 	ret_val = igb_reset_mdicnfg_82580(hw);
 	if (ret_val)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index c1d72c0..090ee56 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3844,7 +3844,6 @@ bool igb_has_link(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	bool link_active = false;
-	s32 ret_val = 0;
 
 	/* get_link_status is set on LSC (link status) interrupt or
 	 * rx sequence error interrupt.  get_link_status will stay
@@ -3853,16 +3852,11 @@ bool igb_has_link(struct igb_adapter *adapter)
 	 */
 	switch (hw->phy.media_type) {
 	case e1000_media_type_copper:
-		if (hw->mac.get_link_status) {
-			ret_val = hw->mac.ops.check_for_link(hw);
-			link_active = !hw->mac.get_link_status;
-		} else {
-			link_active = true;
-		}
-		break;
+		if (!hw->mac.get_link_status)
+			return true;
 	case e1000_media_type_internal_serdes:
-		ret_val = hw->mac.ops.check_for_link(hw);
-		link_active = hw->mac.serdes_has_link;
+		hw->mac.ops.check_for_link(hw);
+		link_active = !hw->mac.get_link_status;
 		break;
 	default:
 	case e1000_media_type_unknown:
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 7e8c477..5a54e3d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -97,14 +97,14 @@ static cycle_t igb_ptp_read_82580(const struct cyclecounter *cc)
 {
 	struct igb_adapter *igb = container_of(cc, struct igb_adapter, cc);
 	struct e1000_hw *hw = &igb->hw;
+	u32 lo, hi;
 	u64 val;
-	u32 lo, hi, jk;
 
 	/* The timestamp latches on lowest register read. For the 82580
 	 * the lowest register is SYSTIMR instead of SYSTIML.  However we only
 	 * need to provide nanosecond resolution, so we just ignore it.
 	 */
-	jk = rd32(E1000_SYSTIMR);
+	rd32(E1000_SYSTIMR);
 	lo = rd32(E1000_SYSTIML);
 	hi = rd32(E1000_SYSTIMH);
 
@@ -118,13 +118,13 @@ static cycle_t igb_ptp_read_82580(const struct cyclecounter *cc)
 static void igb_ptp_read_i210(struct igb_adapter *adapter, struct timespec *ts)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	u32 sec, nsec, jk;
+	u32 sec, nsec;
 
 	/* The timestamp latches on lowest register read. For I210/I211, the
 	 * lowest register is SYSTIMR. Since we only need to provide nanosecond
 	 * resolution, we can ignore it.
 	 */
-	jk = rd32(E1000_SYSTIMR);
+	rd32(E1000_SYSTIMR);
 	nsec = rd32(E1000_SYSTIML);
 	sec = rd32(E1000_SYSTIMH);
 
-- 
1.8.3.1

^ permalink raw reply related

* [net-next v2 01/15] igb: Reset the link when EEE setting changed
From: Jeff Kirsher @ 2013-08-22  9:49 UTC (permalink / raw)
  To: davem; +Cc: Akeem G Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1377164991-27988-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>

This patch resets the link, if link is up - whenever users enable or disable EEE

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 85fe7b5..6d861a5 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2672,7 +2672,9 @@ static int igb_set_eee(struct net_device *netdev,
 		igb_set_eee_i350(hw);
 
 		/* reset link */
-		if (!netif_running(netdev))
+		if (netif_running(netdev))
+			igb_reinit_locked(adapter);
+		else
 			igb_reset(adapter);
 	}
 
-- 
1.8.3.1

^ permalink raw reply related

* [net-next v2 00/15][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-08-22  9:49 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann, sergei.shtylyov,
	bhutchings

This series contains updates to igb and e1000e.

Akeem provides 3 igb patches, the first resets the link when EEE is enabled
or disabled if the link is up.  His second patch changes a register read
which normally stores of the read value to "just-read" so that hardware
can accurately latch the register read.  Lastly, he adds rcu_lock to avoid
a possible race condition with igb_update_stats function.

Mitch provides a fix for SR-IOV, where MSI-X interrupts are required, so
make sure that MSI-X is enabled before allowing the user to turn on SR-IOV.

Alex's igb patch make it so that we limit the lower bound for max_frame_size
to the size of a standard Ethernet frame.  This allows for feature parity
with other Intel based drivers such as ixgbe.

Carolyn adds a SKU for a flashless i210 device and a fix for get_fw_version()
so that it works for all parts for igb.  In addition, she has 2 igb patches
to refactor NVM code to accommodate devices with no flash.  Lastly, she
adds code to check for the failure of pci_disable_link_state() to attempt
to work around a problem found with some systems.

Laura provides the remaining 2 igb patches.  One removing the hard-coded
value for the size of the RETA indirection table, and creates a macro instead
for the RETA indirection table.  The second adds the ethtool callbacks
necessary to change the RETA indirection table from userspace.

Bruce fixes a whitespace issue in a recent commit and resolves a jiffies
comparison warning by using time_after().

Li provides a fix for e1000e to avoid a kernel crash on shutdown by adding
one more check in e1000e_shutdown().  This is due to e1000e_shutdown()
trying to clear correctable errors on the upstream P2P bridge, when under
some cases we do not have the upstream P2P bridge.

v2:
 - fixed patch 11 conditional statement from < to <= based on feedback
   from Ben Hutchings
 - fixed patch 12 patch description (adding the commit summary) based
   on feedback from Sergei Shtylyov

The following are changes since commit f8825669b3165bbf4380eabfd641f693d02af866:
  net: phy: mdio-octeon: use platform_set_drvdata()
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Akeem G Abodunrin (3):
  igb: Reset the link when EEE setting changed
  igb: Read register for latch_on without return value
  igb: Added rcu_lock to avoid race

Alexander Duyck (1):
  igb: Update MTU so that it is always at least a standard frame size

Bruce Allan (2):
  e1000e: cleanup whitespace in recent commit
  e1000e: resolve checkpatch JIFFIES_COMPARISON warning

Carolyn Wyborny (5):
  igb: Refactor of init_nvm_params
  igb: Refactor NVM read functions to accommodate devices with no flash
  igb: Add device support for flashless SKU of i210 device
  igb: Fix get_fw_version function for all parts
  e1000e: Add code to check for failure of pci_disable_link_state call

Laura Mihaela Vasilescu (2):
  igb: Add macro for size of RETA indirection table
  igb: Expose RSS indirection table for ethtool

Li Zhang (1):
  e1000e: Avoid kernel crash during shutdown

Mitch A Williams (1):
  igb: don't allow SR-IOV without MSI-X

 drivers/net/ethernet/intel/e1000e/ethtool.c    |   2 +-
 drivers/net/ethernet/intel/e1000e/hw.h         |   3 +-
 drivers/net/ethernet/intel/e1000e/netdev.c     |  92 ++++++++++-----
 drivers/net/ethernet/intel/igb/e1000_82575.c   | 117 ++++++++-----------
 drivers/net/ethernet/intel/igb/e1000_defines.h |  34 ++++--
 drivers/net/ethernet/intel/igb/e1000_hw.h      |   3 +
 drivers/net/ethernet/intel/igb/e1000_i210.c    | 155 ++++++++++++++++---------
 drivers/net/ethernet/intel/igb/e1000_i210.h    |   5 +-
 drivers/net/ethernet/intel/igb/e1000_nvm.c     |  80 +++++++++----
 drivers/net/ethernet/intel/igb/e1000_nvm.h     |   1 +
 drivers/net/ethernet/intel/igb/igb.h           |   5 +
 drivers/net/ethernet/intel/igb/igb_ethtool.c   | 106 ++++++++++++++++-
 drivers/net/ethernet/intel/igb/igb_main.c      | 105 ++++++++++-------
 drivers/net/ethernet/intel/igb/igb_ptp.c       |   8 +-
 14 files changed, 476 insertions(+), 240 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* (unknown), 
From: Wajeeha Ahmad @ 2013-08-22  9:29 UTC (permalink / raw)


Your Email ID has been picked by the British Telecom Promotions as a lucky Person of a lump sum of 1,000,000.00. To Claim send info To Email: (btcenter003@outlook.com)

^ permalink raw reply

* Re: Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700DeviceDriver into the Linux Kernel
From: liujunliang_ljl @ 2013-08-22  9:23 UTC (permalink / raw)
  To: Joe Perches, horms, davem
  Cc: Francois Romieu, gregkh, sunhecheng, linux-usb, netdev,
	linux-kernel
In-Reply-To: <201308211807295009698@163.com>

Dear all :

		Thanks all of you about your attentions and suggestions, and I think firstly I should read the documentations carefully.

		and then commiting the patch. 

		Thanks all of you for your help and welcome all of you to come to china.

		Thanks again.



2013-08-22 



liujunliang_ljl 



发件人: Joe Perches 
发送时间: 2013-08-21  19:34:10 
收件人: liujunliang_ljl 
抄送: Francois Romieu; gregkh; sunhecheng; linux-usb; netdev; linux-kernel 
主题: Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700DeviceDriver into the Linux Kernel 
 
On Wed, 2013-08-21 at 18:07 +0800, liujunliang_ljl wrote:
>  Thanks a lot and I have been fixed all the problems mentioned above. please check the following patch and thanks again. 
Just trivial comments below:
> diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
[]
> +static void sr_write_reg_async(struct usbnet *dev, u8 reg, u8 value)
> +{
> + usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
> + value, reg, NULL, 0);
> +}
> +
> +static int sr_share_read_word(struct usbnet *dev, int phy, u8 reg, __le16 *value)
> +{
[]
> + netdev_dbg(dev->net, "read shared %d 0x%02x returned 0x%04x, %d\n",
> +        phy, reg, *value, ret);
You have a lot of code that uses inconsistent
indentation.  Code in drivers/net and drivers/usb/net
generally prefers to use alignment to parenthesis for
multi-line statements
The first could use
usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
       value, reg, NULL, 0);
and the second
netdev_dbg(dev->net, "read shared %d 0x%02x returned 0x%04x, %d\n",
   phy, reg, *value, ret);
Maximal use of 8 space indentation tabs followed by
minimal spaces.
There are many of these above.
> +static int sr9700_get_eeprom(struct net_device *net, struct ethtool_eeprom *eeprom, u8 *data)
[]
> + for (i = 0; i < eeprom->len / 2; i++)
> + ret = sr_read_eeprom_word(dev, eeprom->offset / 2 + i, &ebuf[i]);
One too many tabs for the second line, a few of these...
[]
> +static int sr_mdio_read(struct net_device *netdev, int phy_id, int loc)
[]
> + if (rc == 1)
> + return le16_to_cpu(res) | BMSR_LSTATUS;
> + else
> + return le16_to_cpu(res) & ~BMSR_LSTATUS;
The code below the returns here is unreachable.
> +
> + netdev_dbg(dev->net,
> +        "sr_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
> +        phy_id, loc, le16_to_cpu(res));
> +
> + return le16_to_cpu(res);
> +}
You might try to use scripts/checkpatch.pl --strict if you
care about these.  It should flag most of these coding
style inconsistencies.

^ permalink raw reply

* Re: Re: [PATCH-SR9700] Merge USB 1.1 Ethernet Adapter SR9700 DeviceDriver into the Linux Kernel
From: Simon Horman @ 2013-08-22  8:45 UTC (permalink / raw)
  To: liujunliang_ljl
  Cc: Ben Hutchings, gregkh, linux-usb, netdev, linux-kernel,
	sunhecheng
In-Reply-To: <201308211802427961789@163.com>

On Wed, Aug 21, 2013 at 06:02:45PM +0800, liujunliang_ljl wrote:
> Dear Ben :
> 
> 		1,  please give me email address of David Miller, and Thanks a lot.

You can find him in the MAINTAINERS file.
He should also show up if you use scripts/get_maintainer.pl

^ permalink raw reply

* Re: ADSL/ATM linklayer tc shaping regression fix commits for stable
From: Jesper Dangaard Brouer @ 2013-08-22  8:41 UTC (permalink / raw)
  To: David Miller
  Cc: brouer, gregkh, stable, netdev, linux-kernel, Paul Gortmaker,
	Daniel Borkmann
In-Reply-To: <20130822.012348.1370012681436362922.davem@davemloft.net>

On Thu, 22 Aug 2013 01:23:48 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Thu, 22 Aug 2013 10:04:26 +0200
> 
> > So, for future reference:
> > 
> > Stable patches for the networking tree, I should:
> > 1) check http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
> >    to see if my patch is already on your stable queue
> > 2) if not, ask you and cc netdev@vger.kernel.org
> > 3) you will handle the interaction with Greg and stable@vger.kernel.org
> > 
> > Correct?
> 
> Yes, and that's how it's been with the networking for years.

Yes, and in the future, documentation of this will be available here:
 Documentation/networking/netdev-FAQ.txt

Right now is only available via net-next:
 https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/Documentation/networking/netdev-FAQ.txt
or
 https://lwn.net/Articles/559211/

Guess, we should add a pointer to this new doc from
Documentation/stable_kernel_rules.txt once it hits Linus'es tree.

Now I cannot find any more ways to spell-it-out, for people searching google ;-)
-- 
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

* Re: [PATCH] driver:net:stmmac: Disable DMA store and forward mode if platform data force_sf_dma_mode is negative.
From: Sonic Zhang @ 2013-08-22  8:40 UTC (permalink / raw)
  To: Giuseppe CAVALLARO; +Cc: netdev, adi-buildroot-devel, Sonic Zhang
In-Reply-To: <52122BDC.5030500@st.com>

Hi Peppe,

On Mon, Aug 19, 2013 at 10:29 PM, Giuseppe CAVALLARO
<peppe.cavallaro@st.com> wrote:
> On 8/19/2013 12:51 PM, Sonic Zhang wrote:
>>
>> Hi Giuseppe,
>>
>> On Mon, Aug 19, 2013 at 3:45 PM, Giuseppe CAVALLARO
>> <peppe.cavallaro@st.com> wrote:
>>>
>>> On 8/19/2013 9:31 AM, Sonic Zhang wrote:
>>>>
>>>>
>>>> Hi Giuseppe,
>>>>
>>>> On Mon, Aug 19, 2013 at 2:03 PM, Giuseppe CAVALLARO
>>>> <peppe.cavallaro@st.com> wrote:
>>>>>
>>>>>
>>>>> Hello Sonic
>>>>>
>>>>>
>>>>> On 8/15/2013 9:37 AM, Sonic Zhang wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> From: Sonic Zhang <sonic.zhang@analog.com>
>>>>>>
>>>>>> Some synopsys ip implementation doesn't support DMA store and forward
>>>>>> mode,
>>>>>> such as BF60x. So, define force_sf_dma_mode negative to use DMA
>>>>>> thresholds
>>>>>> only.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> I think that you should not pass the force_sf_dma_mode platform field
>>>>> at all (and it doesn't make sense to force it as negative).
>>>>> To use the threshold you should reset tx_coe. In fact, your HW cannot
>>>>> perform the Hw csum if SF is not available.
>>>>> Note that, the HW cap register (if available) can override (set/reset)
>>>>>    tx_coe.
>>>>
>>>>
>>>>
>>>> Even if I reset tx_coe, the SF mode is still set to RX DMA in current
>>>> stmmac_dma_operation_mode(). SF mode is not supported in both RX DMA
>>>> and TX DMA in Blackfin MAC.
>>>
>>>
>>>
>>> yes this is true. the SF is always set for the RX path because I have
>>> never had and known HW w/o RX csum (since the 209).
>>>
>>> So I think the code could be improved to disable/enable the SF also for
>>> the RX path.
>>>
>>>
>>>>>
>>>>> I tested, long time ago, this scenario on old mac w/o HW cap register
>>>>> and w/o SF.
>>>>
>>>>
>>>>
>>>> Blackfin synopsys MAC IP has the HW cap register with tx_coe set to 1,
>>>> but the HW tx_coe doesn't really work. I have the other patch to
>>>> disable HW tx_coe in board file and override the HW cap register.
>>>
>>>
>>>
>>> AFAIK the HW shouldn't be able to perform the csum in HW w/o SF.
>>>
>>> Maybe, an easy way could be to use a new field to force the threshold
>>> mode. This should also remove the csum in HW (IMO) and program the
>>> DMA operation register.
>>>
>>
>> The problem is that HW RX csum works perfectly on Blackfin MAC
>> although the SF mode is not supported in RX DMA.
>
>
> hmm maybe we should ask SYNP. I'll try.
>
>
>> I may need 2 platform
>> fields to force RX DMA threshold and disable HW tx_coe. One field
>> doesn't cover both cases well.
>
>
> concerning tx_coe, I had added it to inform the stmmac that the HW
> was (or not) able to do the csum in hw. This was true on chip w/o
> the HW cap register. If you have the HW cap reg so let me assume
> there is another problem. For example, I worked (time ago) on an HW
> where the cap reg declared that the mac was able to perform the csum
> in hw but, after debugging it, I discovered that the problem was on
> SG. I mean, the HW was able to do the csum in HW but had problems on
> fragmented frame (I mean, frames that were split in one more
> descriptor).
>
> I will send you the patch I used to fix that... maybe you are in the
> same scenario.
>
> Concerning the threshold, just to avoid to complicate the code, we could
> keep force_sf_dma_mode and add force_thresh_dma_mode that force both
> rx and tx. I do not want to remove the force_sf_dma_mode that is also
> used on some platforms (AFAIK).
> Do not forget to update the stmmac.txt and devicetree support
> What do you think?
> I also think that the patch should be prepared on top of net-next.

OK, I will use the new flag and update document and device tree as well.
When can you send me your fragmented frame patch?

Regards,

Sonic

^ permalink raw reply

* Re: Performance regression 3.11 with macvlan between 2 linux guests (bisected)
From: David Miller @ 2013-08-22  8:29 UTC (permalink / raw)
  To: borntraeger; +Cc: vyasevic, netdev, mst, mjrosato, jasowang
In-Reply-To: <5215CA84.8070906@de.ibm.com>

From: Christian Borntraeger <borntraeger@de.ibm.com>
Date: Thu, 22 Aug 2013 10:23:32 +0200

> On 21/08/13 20:12, Vlad Yasevich wrote:
> 
>>>
>>> A corrected patch has been sent upstream.  We take into consideration
>>> the features on the target device that the user/vm has specified.
>>> If the VM has enabled the TSO flags, then nothing will happen to the
>>> GSO packet.  However, if the TSO flag is off, segmentation will be
>>> performed.
>> 
>> Particularly.  This commit should fix the issue:
>> commit a567dd6252263c8147b7269df5d03d9e31463e11
>>  macvtap: simplify usage of tap_features
> 
> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> The patch is still in net.git, but not in Linus git. Are we going to push this
> for 3.11?

Of course.  Anything in 'net' is intended to make it into Linus's tree.

I generally push things to Linus every week or two.

^ permalink raw reply

* Re: ADSL/ATM linklayer tc shaping regression fix commits for stable
From: David Miller @ 2013-08-22  8:23 UTC (permalink / raw)
  To: brouer; +Cc: gregkh, stable, netdev, linux-kernel
In-Reply-To: <20130822100426.39be319d@redhat.com>

From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Thu, 22 Aug 2013 10:04:26 +0200

> So, for future reference:
> 
> Stable patches for the networking tree, I should:
> 1) check http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
>    to see if my patch is already on your stable queue
> 2) if not, ask you and cc netdev@vger.kernel.org
> 3) you will handle the interaction with Greg and stable@vger.kernel.org
> 
> Correct?

Yes, and that's how it's been with the networking for years.

^ permalink raw reply

* Re: Performance regression 3.11 with macvlan between 2 linux guests (bisected)
From: Christian Borntraeger @ 2013-08-22  8:23 UTC (permalink / raw)
  To: vyasevic
  Cc: David S. Miller, netdev, Michael S. Tsirkin, Matthew Rosato,
	Jason Wang
In-Reply-To: <521502F6.5030005@redhat.com>

On 21/08/13 20:12, Vlad Yasevich wrote:

>>
>> A corrected patch has been sent upstream.  We take into consideration
>> the features on the target device that the user/vm has specified.
>> If the VM has enabled the TSO flags, then nothing will happen to the
>> GSO packet.  However, if the TSO flag is off, segmentation will be
>> performed.
> 
> Particularly.  This commit should fix the issue:
> commit a567dd6252263c8147b7269df5d03d9e31463e11
>  macvtap: simplify usage of tap_features

Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>

The patch is still in net.git, but not in Linus git. Are we going to push this
for 3.11?

^ permalink raw reply

* Re: ADSL/ATM linklayer tc shaping regression fix commits for stable
From: Jesper Dangaard Brouer @ 2013-08-22  8:04 UTC (permalink / raw)
  To: David Miller; +Cc: gregkh, stable, netdev, linux-kernel, brouer
In-Reply-To: <20130821.112023.744177612387539958.davem@davemloft.net>

On Wed, 21 Aug 2013 11:20:23 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Wed, 21 Aug 2013 14:03:50 +0200
> 
> > 
> > On Tue, 20 Aug 2013 08:16:50 -0700
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >> On Tue, Aug 20, 2013 at 01:26:29PM +0200, Jesper Dangaard Brouer wrote:
> > [...]
> >  
> >> If there are issues with 3.10, that's a different story.
> >> 
> >> > Refactor improvements (v3.11-rc1):
> >> >  commit 130d3d68b52 (net_sched: psched_ratecfg_precompute() improvements)
> > 
> > Needed because it fixes the accuracy of the rate calc.
> > 
> >> > The linklayer ATM/ADSL fix, reached 3.11-rc6:
> >> >  commit 8a8e3d84b17 (net_sched: restore "linklayer atm" handling)
> > 
> > Needed because we broke userspace interface.
> > 
> >> David sends me the networking patches for the stable tree, and if he
> >> thinks these are applicable, then I'll take them.
> > 
> > DaveM, please?  What do you want me to do, submit this req/patches somehow?
> > 
> > (Can see that is not currently on your stable queue via:
> > http://patchwork.ozlabs.org/bundle/davem/stable/?state=*)
> 
> I'll put it there, you just have to formally ask me to do it that's
> all.

Thanks, I assume that this was formally enough and you are taking the patches.

So, for future reference:

Stable patches for the networking tree, I should:
1) check http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
   to see if my patch is already on your stable queue
2) if not, ask you and cc netdev@vger.kernel.org
3) you will handle the interaction with Greg and stable@vger.kernel.org

Correct?


> Asking Greg to do it directly is a complete waste of time. :-)

I'll remember that.  I was just following Documentation/stable_kernel_rules.txt,
I guess I should have known better as all my previous stable patches
have automagically been picked up and processed by you :-)

-- 
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

* pull-request: can-next 2013-08-22
From: Marc Kleine-Budde @ 2013-08-22  7:49 UTC (permalink / raw)
  To: netdev; +Cc: linux-can@vger.kernel.org, David Miller, kernel@pengutronix.de

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

Hello David,

another pull-request for net-next. It consists of two patches by Libo
Chen, the at91 and flexcan driver make use of platform_set_drvdata()
rather than open coding it. Chen Gang improves the error checking in
the c_can_platform driver's probe function.

regards,
Marc

---

The following changes since commit f8825669b3165bbf4380eabfd641f693d02af866:

  net: phy: mdio-octeon: use platform_set_drvdata() (2013-08-21 20:51:11 -0700)

are available in the git repository at:

  git://gitorious.org/linux-can/linux-can-next.git for-davem

for you to fetch changes up to d75ea942b360690a380da8012a51eaf6a6ebb1b1:

  can: flexcan: use platform_set_drvdata() (2013-08-22 09:40:20 +0200)

----------------------------------------------------------------
Chen Gang (1):
      can: c_can: fix error checking of priv->instance in probe()

Libo Chen (2):
      can: at91_can: use platform_set_drvdata()
      can: flexcan: use platform_set_drvdata()

 drivers/net/can/at91_can.c             | 2 +-
 drivers/net/can/c_can/c_can_platform.c | 2 +-
 drivers/net/can/flexcan.c              | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* Re: [PATCH RFC 2/2] ipv6: Add support for IPsec virtual tunnel interfaces
From: Fan Du @ 2013-08-22  7:47 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: netdev
In-Reply-To: <20130819062730.GP26773@secunet.com>



On 2013年08月19日 14:27, Steffen Klassert wrote:
> This patch adds IPv6  support for IPsec virtual tunnel interfaces
> (vti). IPsec virtual tunnel interfaces provide a routable interface
> for IPsec tunnel endpoints.
>
> Signed-off-by: Steffen Klassert<steffen.klassert@secunet.com>
> ---
>   net/ipv6/Kconfig   |   11 +
>   net/ipv6/Makefile  |    1 +
>   net/ipv6/ip6_vti.c | 1092 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 1104 insertions(+)
>   create mode 100644 net/ipv6/ip6_vti.c
>
> diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
> index 11b13ea..e1a8d90 100644
> --- a/net/ipv6/Kconfig
> +++ b/net/ipv6/Kconfig
> @@ -153,6 +153,17 @@ config INET6_XFRM_MODE_ROUTEOPTIMIZATION
>   	---help---
>   	  Support for MIPv6 route optimization mode.
>
> +config IPV6_VTI
> +tristate "Virtual (secure) IPv6: tunneling"
> +	select IPV6_TUNNEL
> +	depends on INET6_XFRM_MODE_TUNNEL
> +	---help---
> +	Tunneling means encapsulating data of one protocol type within
> +	another protocol and sending it over a channel that understands the
> +	encapsulating protocol. This can be used with xfrm mode tunnel to give
> +	the notion of a secure tunnel for IPSEC and then use routing protocol
> +	on top.
> +
>   config IPV6_SIT
>   	tristate "IPv6: IPv6-in-IPv4 tunnel (SIT driver)"
>   	select INET_TUNNEL
> diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
> index 470a9c0..17bb830 100644
> --- a/net/ipv6/Makefile
> +++ b/net/ipv6/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_INET6_XFRM_MODE_BEET) += xfrm6_mode_beet.o
>   obj-$(CONFIG_IPV6_MIP6) += mip6.o
>   obj-$(CONFIG_NETFILTER)	+= netfilter/
>
> +obj-$(CONFIG_IPV6_VTI) += ip6_vti.o
>   obj-$(CONFIG_IPV6_SIT) += sit.o
>   obj-$(CONFIG_IPV6_TUNNEL) += ip6_tunnel.o
>   obj-$(CONFIG_IPV6_GRE) += ip6_gre.o
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> new file mode 100644
> index 0000000..fb0b6b0
> --- /dev/null
> +++ b/net/ipv6/ip6_vti.c
> @@ -0,0 +1,1092 @@
> +/*
> + *	IPv6 virtual tunneling interface
> + *
> + *	Copyright (C) 2013 secunet Security Networks AG
> + *
> + *	Author:
> + *	Steffen Klassert<steffen.klassert@secunet.com>
> + *
> + *	Based on:
> + *	net/ipv6/ip6_tunnel.c
> + *
> + *	This program is free software; you can redistribute it and/or
> + *	modify it under the terms of the GNU General Public License
> + *	as published by the Free Software Foundation; either version
> + *	2 of the License, or (at your option) any later version.
> + */
> +
> +#include<linux/module.h>
> +#include<linux/capability.h>
> +#include<linux/errno.h>
> +#include<linux/types.h>
> +#include<linux/sockios.h>
> +#include<linux/icmp.h>
> +#include<linux/if.h>
> +#include<linux/in.h>
> +#include<linux/ip.h>
> +#include<linux/if_tunnel.h>
> +#include<linux/net.h>
> +#include<linux/in6.h>
> +#include<linux/netdevice.h>
> +#include<linux/if_arp.h>
> +#include<linux/icmpv6.h>
> +#include<linux/init.h>
> +#include<linux/route.h>
> +#include<linux/rtnetlink.h>
> +#include<linux/netfilter_ipv6.h>
> +#include<linux/slab.h>
> +#include<linux/hash.h>
> +
> +#include<asm/uaccess.h>
> +#include<linux/atomic.h>
> +
> +#include<net/icmp.h>
> +#include<net/ip.h>
> +#include<net/ip_tunnels.h>
> +#include<net/ipv6.h>
> +#include<net/ip6_route.h>
> +#include<net/addrconf.h>
> +#include<net/ip6_tunnel.h>
> +#include<net/xfrm.h>
> +#include<net/net_namespace.h>
> +#include<net/netns/generic.h>
> +
> +#define IPV6_TCLASS_MASK (IPV6_FLOWINFO_MASK&  ~IPV6_FLOWLABEL_MASK)
> +
> +#define HASH_SIZE_SHIFT  5
> +#define HASH_SIZE (1<<  HASH_SIZE_SHIFT)
> +
> +static u32 HASH(const struct in6_addr *addr1, const struct in6_addr *addr2)
> +{
> +	u32 hash = ipv6_addr_hash(addr1) ^ ipv6_addr_hash(addr2);
> +
> +	return hash_32(hash, HASH_SIZE_SHIFT);
> +}
> +
> +static int vti6_dev_init(struct net_device *dev);
> +static void vti6_dev_setup(struct net_device *dev);
> +static struct rtnl_link_ops vti6_link_ops __read_mostly;
> +
> +static int vti6_net_id __read_mostly;
> +struct vti6_net {
> +	/* the IPv6 tunnel fallback device */
> +	struct net_device *fb_tnl_dev;
> +	/* lists for storing tunnels in use */
> +	struct ip6_tnl __rcu *tnls_r_l[HASH_SIZE];
> +	struct ip6_tnl __rcu *tnls_wc[1];
> +	struct ip6_tnl __rcu **tnls[2];
> +};
> +
> +static struct net_device_stats *vti6_get_stats(struct net_device *dev)
> +{
> +	struct pcpu_tstats sum = { 0 };
> +	int i;
> +
> +	for_each_possible_cpu(i) {
> +		const struct pcpu_tstats *tstats = per_cpu_ptr(dev->tstats, i);
> +
> +		sum.rx_packets += tstats->rx_packets;
> +		sum.rx_bytes   += tstats->rx_bytes;
> +		sum.tx_packets += tstats->tx_packets;
> +		sum.tx_bytes   += tstats->tx_bytes;
> +	}
> +	dev->stats.rx_packets = sum.rx_packets;
> +	dev->stats.rx_bytes   = sum.rx_bytes;
> +	dev->stats.tx_packets = sum.tx_packets;
> +	dev->stats.tx_bytes   = sum.tx_bytes;
> +	return&dev->stats;
> +}
> +
> +#define for_each_vti6_tunnel_rcu(start) \
> +	for (t = rcu_dereference(start); t; t = rcu_dereference(t->next))
> +
> +/**
> + * vti6_tnl_lookup - fetch tunnel matching the end-point addresses
> + *   @net: network namespace
> + *   @remote: the address of the tunnel exit-point
> + *   @local: the address of the tunnel entry-point
> + *
> + * Return:
> + *   tunnel matching given end-points if found,
> + *   else fallback tunnel if its device is up,
> + *   else %NULL
> + **/
> +static struct ip6_tnl *
> +vti6_tnl_lookup(struct net *net, const struct in6_addr *remote,
> +		const struct in6_addr *local)
> +{
> +	unsigned int hash = HASH(remote, local);
> +	struct ip6_tnl *t;
> +	struct vti6_net *ip6n = net_generic(net, vti6_net_id);
> +
> +	for_each_vti6_tunnel_rcu(ip6n->tnls_r_l[hash]) {
> +		if (ipv6_addr_equal(local,&t->parms.laddr)&&
> +		    ipv6_addr_equal(remote,&t->parms.raddr)&&
> +		    (t->dev->flags&  IFF_UP))
> +			return t;
> +	}
> +	t = rcu_dereference(ip6n->tnls_wc[0]);
> +	if (t&&  (t->dev->flags&  IFF_UP))
> +		return t;
> +
> +	return NULL;
> +}
> +
> +/**
> + * vti6_tnl_bucket - get head of list matching given tunnel parameters
> + *   @p: parameters containing tunnel end-points
> + *
> + * Description:
> + *   vti6_tnl_bucket() returns the head of the list matching the
> + *&struct in6_addr entries laddr and raddr in @p.
> + *
> + * Return: head of IPv6 tunnel list
> + **/
> +static struct ip6_tnl __rcu **
> +vti6_tnl_bucket(struct vti6_net *ip6n, const struct __ip6_tnl_parm *p)
> +{
> +	const struct in6_addr *remote =&p->raddr;
> +	const struct in6_addr *local =&p->laddr;
> +	unsigned int h = 0;
> +	int prio = 0;
> +
> +	if (!ipv6_addr_any(remote) || !ipv6_addr_any(local)) {
> +		prio = 1;
> +		h = HASH(remote, local);
> +	}
> +	return&ip6n->tnls[prio][h];
> +}
> +
> +static void
> +vti6_tnl_link(struct vti6_net *ip6n, struct ip6_tnl *t)
> +{
> +	struct ip6_tnl __rcu **tp = vti6_tnl_bucket(ip6n,&t->parms);
> +
> +	rcu_assign_pointer(t->next , rtnl_dereference(*tp));
> +	rcu_assign_pointer(*tp, t);
> +}
> +
> +static void
> +vti6_tnl_unlink(struct vti6_net *ip6n, struct ip6_tnl *t)
> +{
> +	struct ip6_tnl __rcu **tp;
> +	struct ip6_tnl *iter;
> +
> +	for (tp = vti6_tnl_bucket(ip6n,&t->parms);
> +	     (iter = rtnl_dereference(*tp)) != NULL;
> +	     tp =&iter->next) {
> +		if (t == iter) {
> +			rcu_assign_pointer(*tp, t->next);
> +			break;
> +		}
> +	}
> +}
> +
> +static void vti6_dev_free(struct net_device *dev)
> +{
> +	free_percpu(dev->tstats);
> +	free_netdev(dev);
> +}
> +
> +static int vti6_tnl_create2(struct net_device *dev)
> +{
> +	struct ip6_tnl *t = netdev_priv(dev);
> +	struct net *net = dev_net(dev);
> +	struct vti6_net *ip6n = net_generic(net, vti6_net_id);
> +	int err;
> +
> +	err = vti6_dev_init(dev);
> +	if (err<  0)
> +		goto out;
> +
> +	err = register_netdevice(dev);
> +	if (err<  0)
> +		goto out;
> +
> +	strcpy(t->parms.name, dev->name);
> +	dev->rtnl_link_ops =&vti6_link_ops;
> +
> +	dev_hold(dev);
> +	vti6_tnl_link(ip6n, t);
> +
> +	return 0;
> +
> +out:
> +	return err;
> +}
> +static struct ip6_tnl *vti6_tnl_create(struct net *net, struct __ip6_tnl_parm *p)
> +{
> +	struct net_device *dev;
> +	struct ip6_tnl *t;
> +	char name[IFNAMSIZ];
> +	int err;
> +
> +	if (p->name[0])
> +		strlcpy(name, p->name, IFNAMSIZ);
> +	else
> +		sprintf(name, "ip6_vti%%d");
> +
> +	dev = alloc_netdev(sizeof (*t), name, vti6_dev_setup);
> +	if (dev == NULL)
> +		goto failed;
> +
> +	dev_net_set(dev, net);
> +
> +	t = netdev_priv(dev);
> +	t->parms = *p;
> +	t->net = dev_net(dev);
> +
> +	err = vti6_tnl_create2(dev);
> +	if (err<  0)
> +		goto failed_free;
> +
> +	return t;
> +
> +failed_free:
> +	vti6_dev_free(dev);
> +failed:
> +	return NULL;
> +}
> +
> +/**
> + * vti6_locate - find or create tunnel matching given parameters
> + *   @net: network namespace
> + *   @p: tunnel parameters
> + *   @create: != 0 if allowed to create new tunnel if no match found
> + *
> + * Description:
> + *   vti6_locate() first tries to locate an existing tunnel
> + *   based on @parms. If this is unsuccessful, but @create is set a new
> + *   tunnel device is created and registered for use.
> + *
> + * Return:
> + *   matching tunnel or NULL
> + **/
> +static struct ip6_tnl *vti6_locate(struct net *net, struct __ip6_tnl_parm *p, int create)
> +{
> +	const struct in6_addr *remote =&p->raddr;
> +	const struct in6_addr *local =&p->laddr;
> +	struct ip6_tnl __rcu **tp;
> +	struct ip6_tnl *t;
> +	struct vti6_net *ip6n = net_generic(net, vti6_net_id);
> +
> +	for (tp = vti6_tnl_bucket(ip6n, p);
> +	     (t = rtnl_dereference(*tp)) != NULL;
> +	     tp =&t->next) {
> +		if (ipv6_addr_equal(local,&t->parms.laddr)&&
> +		    ipv6_addr_equal(remote,&t->parms.raddr))
> +			return t;
> +	}
> +	if (!create)
> +		return NULL;
> +	return vti6_tnl_create(net, p);
> +}
> +
> +/**
> + * vti6_dev_uninit - tunnel device uninitializer
> + *   @dev: the device to be destroyed
> + *
> + * Description:
> + *   vti6_dev_uninit() removes tunnel from its list
> + **/
> +static void
> +vti6_dev_uninit(struct net_device *dev)
> +{
> +	struct ip6_tnl *t = netdev_priv(dev);
> +	struct net *net = dev_net(dev);
> +	struct vti6_net *ip6n = net_generic(net, vti6_net_id);
> +
> +	if (dev == ip6n->fb_tnl_dev)
> +		RCU_INIT_POINTER(ip6n->tnls_wc[0], NULL);
> +	else
> +		vti6_tnl_unlink(ip6n, t);
> +	ip6_tnl_dst_reset(t);
> +	dev_put(dev);
> +}
> +
> +/**
> + * vti6_tnl_err - tunnel error handler
> + *
> + * Description:
> + *   vti6_err() handle errors in the tunnel.
> + **/
> +static int
> +vti6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> +	 u8 type, u8 code, int offset, __be32 info)
> +{
> +	struct net *net = dev_net(skb->dev);
> +	const struct ipv6hdr *ipv6h = (const struct ipv6hdr *) skb->data;
> +	struct ip6_tnl *t;
> +	int err = -ENOENT;
> +
> +	rcu_read_lock();
> +	if ((t = vti6_tnl_lookup(net,&ipv6h->daddr,
> +				&ipv6h->saddr)) == NULL)
> +		goto out;
> +
> +	if (t->parms.proto != IPPROTO_IPV6&&  t->parms.proto != 0)
> +		goto out;
> +
> +	err = 0;
> +
> +	switch (type) {
> +	case ICMPV6_DEST_UNREACH:
> +	case ICMPV6_TIME_EXCEED:
> +	case ICMPV6_PARAMPROB:
> +	break;
> +	case ICMPV6_PKT_TOOBIG:
> +		ip6_update_pmtu(skb, net, info, 0, 0);
> +	}
> +
> +
> +out:
> +	rcu_read_unlock();
> +	return err;
> +}
> +
> +static int vti6_rcv(struct sk_buff *skb)
> +{
> +	struct ip6_tnl *t;
> +	const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> +
> +	rcu_read_lock();
> +
> +	if ((t = vti6_tnl_lookup(dev_net(skb->dev),&ipv6h->saddr,
> +					&ipv6h->daddr)) != NULL) {
> +		struct pcpu_tstats *tstats;
> +
> +		if (t->parms.proto != IPPROTO_IPV6&&  t->parms.proto != 0) {
> +			rcu_read_unlock();
> +			goto discard;
> +		}
> +
> +		if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
> +			rcu_read_unlock();
> +			return 0;
> +		}
> +
> +		if (!ip6_tnl_rcv_ctl(t,&ipv6h->daddr,&ipv6h->saddr)) {
> +			t->dev->stats.rx_dropped++;
> +			rcu_read_unlock();
> +			goto discard;
> +		}
> +
> +		tstats = this_cpu_ptr(t->dev->tstats);
> +		tstats->rx_packets++;
> +		tstats->rx_bytes += skb->len;
> +
> +		skb->mark = 0;
> +		secpath_reset(skb);
> +		skb->dev = t->dev;
> +
> +		rcu_read_unlock();
> +		return 0;
> +	}
> +	rcu_read_unlock();
> +	return 1;
> +
> +discard:
> +	kfree_skb(skb);
> +	return 0;
> +}
> +
> +/**
> + * vti6_addr_conflict - compare packet addresses to tunnel's own
> + *   @t: the outgoing tunnel device
> + *   @hdr: IPv6 header from the incoming packet
> + *
> + * Description:
> + *   Avoid trivial tunneling loop by checking that tunnel exit-point
> + *   doesn't match source of incoming packet.
> + *
> + * Return:
> + *   1 if conflict,
> + *   0 else
> + **/
> +static inline bool
> +vti6_addr_conflict(const struct ip6_tnl *t, const struct ipv6hdr *hdr)
> +{
> +	return ipv6_addr_equal(&t->parms.raddr,&hdr->saddr);
> +}
> +
> +/**
> + * vti6_xmit - send a packet
> + *   @skb: the outgoing socket buffer
> + *   @dev: the outgoing tunnel device
> + **/
> +static int vti6_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct net *net = dev_net(dev);
> +	struct ip6_tnl *t = netdev_priv(dev);
> +	struct net_device_stats *stats =&t->dev->stats;
> +	struct dst_entry *dst = NULL, *ndst = NULL;
> +	struct flowi6 fl6;
> +	struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> +	struct net_device *tdev;
> +	int err = -1;
> +
> +	if ((t->parms.proto != IPPROTO_IPV6&&  t->parms.proto != 0) ||
> +	    !ip6_tnl_xmit_ctl(t) || vti6_addr_conflict(t, ipv6h))
> +		return err;
> +
> +	dst = ip6_tnl_dst_check(t);
> +	if (!dst) {
> +		memcpy(&fl6,&t->fl.u.ip6, sizeof(fl6));
> +
> +		ndst = ip6_route_output(net, NULL,&fl6);
> +
> +		if (ndst->error)
> +			goto tx_err_link_failure;
> +		ndst = xfrm_lookup(net, ndst, flowi6_to_flowi(&fl6), NULL, 0);
> +		if (IS_ERR(ndst)) {
> +			err = PTR_ERR(ndst);
> +			ndst = NULL;
> +			goto tx_err_link_failure;
> +		}
> +		dst = ndst;
> +	}
> +
> +	if (!dst->xfrm || dst->xfrm->props.mode != XFRM_MODE_TUNNEL)
> +		goto tx_err_link_failure;
> +
> +	tdev = dst->dev;
> +
> +	if (tdev == dev) {
> +		stats->collisions++;
> +		net_warn_ratelimited("%s: Local routing loop detected!\n",
> +				     t->parms.name);
> +		goto tx_err_dst_release;
> +	}
> +
> +
> +	skb_dst_drop(skb);
> +	skb_dst_set_noref(skb, dst);
> +
> +	ip6tunnel_xmit(skb, dev);
> +	if (ndst)
> +		ip6_tnl_dst_store(t, ndst);
> +	return 0;
> +tx_err_link_failure:
> +	stats->tx_carrier_errors++;
> +	dst_link_failure(skb);
> +tx_err_dst_release:

Maybe it's necessary to count stats->tx_errors as well.

> +	dst_release(ndst);
> +	return err;
> +}

-- 
浮沉随浪只记今朝笑

--fan

^ permalink raw reply

* Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.
From: Johannes Berg @ 2013-08-22  7:40 UTC (permalink / raw)
  To: Pravin B Shelar; +Cc: netdev, Jesse Gross
In-Reply-To: <1377143892-20763-1-git-send-email-pshelar@nicira.com>

On Wed, 2013-08-21 at 20:58 -0700, Pravin B Shelar wrote:
> netlink dump operations take module as parameter to hold
> reference for entire netlink dump duration.
> Currently it holds ref only on genl module which is not correct
> when we use ops registered to genl from another module.
> Following patch adds module pointer to genl_ops so that netlink
> can hold ref count on it.
> 
> CC: Jesse Gross <jesse@nicira.com>
> CC: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
>  include/net/genetlink.h |   21 ++++++++++++++++++---
>  net/netlink/genetlink.c |   39 ++++++++++++++++++++++++---------------
>  2 files changed, 42 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/genetlink.h b/include/net/genetlink.h
> index 93024a4..7f57b2c 100644
> --- a/include/net/genetlink.h
> +++ b/include/net/genetlink.h
> @@ -119,13 +119,28 @@ struct genl_ops {
>  					 struct netlink_callback *cb);
>  	int		       (*done)(struct netlink_callback *cb);
>  	struct list_head	ops_list;
> +	struct module		*module;
>  };

This may be more correct than my patch, but I'm not sure it's worth
spending the memory. Is there going to be any generic netlink family
that actually puts operations into a different module than the family
itself? I doubt that.

> @@ -605,14 +610,18 @@ static int genl_family_rcv_msg(struct genl_family *family,
>  			genl_unlock();
>  			c.data = ops;
>  			c.dump = genl_lock_dumpit;
> -			if (ops->done)
> -				c.done = genl_lock_done;
> +			c.done = genl_lock_done;
> +			c.module = THIS_MODULE;

THIS_MODULE here is useless, this code is always built-in.

> +			if (!try_module_get(ops->module))
> +				return -EPROTONOSUPPORT;

Why open-code it? You can just point c.module to the ops module here as
well (because generic netlink is built-in) and save yourself the
try_module_get stuff.

johannes

^ permalink raw reply

* Re: [PATCH 1/2] genl: Fix genl dumpit() locking.
From: Johannes Berg @ 2013-08-22  7:36 UTC (permalink / raw)
  To: Pravin B Shelar; +Cc: netdev, Jesse Gross
In-Reply-To: <1377143882-20717-1-git-send-email-pshelar@nicira.com>

On Wed, 2013-08-21 at 20:58 -0700, Pravin B Shelar wrote:
> In case of genl-family with parallel ops off, dumpif() callback
> is expected to run under genl_lock, But commit def3117493eafd9df
> (genl: Allow concurrent genl callbacks.) changed this behaviour
> where only first dumpit() op was called under genl-lock.
> For subsequent dump, only nlk->cb_lock was taken.
> Following patch fixes it by defining locked dumpit() and done()
> callback which takes care of genl-locking.

As I've commented over in the other thread, I really think this is the
wrong thing to do. It was never the case that dumpit() was actually
locked under genl_lock(), and adding it doesn't really help anyone.

johannes

^ permalink raw reply

* [RFC] genetlink: don't allow unloading families while dumping
From: Johannes Berg @ 2013-08-22  7:21 UTC (permalink / raw)
  To: netdev; +Cc: Pravin Shelar, Thomas Graf, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

The netlink code will prevent unloading of a module while
the module is dumping, to prevent the dump() callback from
going away and crashing in the next call. However, generic
netlink just points to its family's dumpit call, which can
be in a different module, without pointing to that module.

Add a struct module pointer to generic netlink families and
pass the pointer to netlink_dump_start().

TODO: move/adjust the documentation

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/genetlink.h | 21 +++++++++++++++++++--
 net/netlink/genetlink.c | 10 +++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 93024a4..0bc3a53 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -42,6 +42,7 @@ struct genl_info;
  * @ops_list: list of all assigned operations
  * @family_list: family list
  * @mcast_groups: multicast groups list
+ * @module: the module containing this family
  */
 struct genl_family {
 	unsigned int		id;
@@ -61,6 +62,7 @@ struct genl_family {
 	struct list_head	ops_list;	/* private */
 	struct list_head	family_list;	/* private */
 	struct list_head	mcast_groups;	/* private */
+	struct module *		module;		/* private */
 };
 
 /**
@@ -121,9 +123,24 @@ struct genl_ops {
 	struct list_head	ops_list;
 };
 
-extern int genl_register_family(struct genl_family *family);
-extern int genl_register_family_with_ops(struct genl_family *family,
+extern int __genl_register_family(struct genl_family *family);
+extern int __genl_register_family_with_ops(struct genl_family *family,
 	struct genl_ops *ops, size_t n_ops);
+
+static inline int genl_register_family(struct genl_family *family)
+{
+	family->module = THIS_MODULE;
+	return __genl_register_family(family);
+}
+
+static inline int genl_register_family_with_ops(struct genl_family *family,
+						struct genl_ops *ops,
+						size_t n_ops)
+{
+	family->module = THIS_MODULE;
+	return __genl_register_family_with_ops(family, ops, n_ops);
+}
+
 extern int genl_unregister_family(struct genl_family *family);
 extern int genl_register_ops(struct genl_family *, struct genl_ops *ops);
 extern int genl_unregister_ops(struct genl_family *, struct genl_ops *ops);
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 2fd6dbe..8bab493 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -374,7 +374,7 @@ EXPORT_SYMBOL(genl_unregister_ops);
  *
  * Return 0 on success or a negative error code.
  */
-int genl_register_family(struct genl_family *family)
+int __genl_register_family(struct genl_family *family)
 {
 	int err = -EINVAL;
 
@@ -430,7 +430,7 @@ errout_locked:
 errout:
 	return err;
 }
-EXPORT_SYMBOL(genl_register_family);
+EXPORT_SYMBOL(__genl_register_family);
 
 /**
  * genl_register_family_with_ops - register a generic netlink family
@@ -457,12 +457,12 @@ EXPORT_SYMBOL(genl_register_family);
  *
  * Return 0 on success or a negative error code.
  */
-int genl_register_family_with_ops(struct genl_family *family,
+int __genl_register_family_with_ops(struct genl_family *family,
 	struct genl_ops *ops, size_t n_ops)
 {
 	int err, i;
 
-	err = genl_register_family(family);
+	err = __genl_register_family(family);
 	if (err)
 		return err;
 
@@ -476,7 +476,7 @@ err_out:
 	genl_unregister_family(family);
 	return err;
 }
-EXPORT_SYMBOL(genl_register_family_with_ops);
+EXPORT_SYMBOL(__genl_register_family_with_ops);
 
 /**
  * genl_unregister_family - unregister generic netlink family
-- 
1.8.4.rc2

^ permalink raw reply related

* Re: [PATCH 2/2] genetlink: convert family dump code to use RCU
From: Johannes Berg @ 2013-08-22  7:17 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: netdev, Thomas Graf
In-Reply-To: <CALnjE+oqN0NC1xmzTRFXOuMb+4L83JJyo5SGqBTvW-KfefE39g@mail.gmail.com>

On Wed, 2013-08-21 at 21:32 -0700, Pravin Shelar wrote:

> I send out a fix which fixes genl locking for dump operation and it
> might fix this issue, Can you try this?

I don't think that fix is correct. I don't have a copy, so let me reply
here.

> http://marc.info/?l=linux-netdev&m=137714389705485&w=2

> In case of genl-family with parallel ops off, dumpif() callback
> is expected to run under genl_lock, 
> But commit def3117493eafd9df
> (genl: Allow concurrent genl callbacks.) changed this behaviour
> where only first dumpit() op was called under genl-lock.

I don't think either of those statements are true - dump() has alway
taken a shortcut in netlink_recvmsg() and doesn't go into genl_rcv(), so
how could your patch have changed locking? Therefore, it can't have been
expected either.

> For subsequent dump, only nlk->cb_lock was taken.
> Following patch fixes it by defining locked dumpit() and done()
> callback which takes care of genl-locking.

This might help for generic netlink itself, but I'm not convinced that
it's really useful for other families. I've fixed bugs like in commit
3a5a423bb958ad22eeccca66c533e85bf69ba10e, which actually made it in
after your commit, but per above I don't think it was actually a problem
introduced by your commit.

johannes

^ permalink raw reply

* RE: [PATCH 4/4] qlcnic: use standard NAPI weights
From: Himanshu Madhani @ 2013-08-22  6:51 UTC (permalink / raw)
  To: Michal Schmidt, David Miller
  Cc: netdev, Eric Dumazet, Rajesh Borundia, Shahed Shaikh,
	Jitendra Kalsaria, Sony Chacko, Sucheta Chakraborty,
	Dept-Eng Linux Driver
In-Reply-To: <1377103771-1590-5-git-send-email-mschmidt@redhat.com>

________________________________________
From: Michal Schmidt [mschmidt@redhat.com]
Sent: Wednesday, August 21, 2013 9:49 AM
To: David Miller
Cc: netdev; Eric Dumazet; Himanshu Madhani; Rajesh Borundia; Shahed Shaikh; Jitendra Kalsaria; Sony Chacko; Sucheta Chakraborty; Dept-Eng Linux Driver
Subject: [PATCH 4/4] qlcnic: use standard NAPI weights

Since commit 82dc3c63 netif_napi_add() produces an error message if
a NAPI poll weight greater than 64 is requested.

qlcnic requests the weight as large as 256 for some of its rings, and
smaller values for other rings. For instance in qlcnic_82xx_napi_add()
I think the intention was to give the tx+rx ring a bigger weight than
to rx-only rings, but it's actually doing the opposite. So I'm assuming
the weights do not really matter much.

Just use the standard NAPI weights for all rings.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
CC: Himanshu Madhani <himanshu.madhani@qlogic.com>
CC: Rajesh Borundia <rajesh.borundia@qlogic.com>
CC: Shahed Shaikh <shahed.shaikh@qlogic.com>
CC: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
CC: Sony Chacko <sony.chacko@qlogic.com>
CC: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
CC: linux-driver@qlogic.com
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h    |  1 -
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 25 ++++++++++---------------
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 3dcc666..47876a5 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -926,7 +926,6 @@ struct qlcnic_ipaddr {
 #define QLCNIC_PCI_REG_MSIX_TBL        0x44
 #define QLCNIC_MSIX_TBL_PGSIZE         4096

-#define QLCNIC_NETDEV_WEIGHT   128
 #define QLCNIC_ADAPTER_UP_MAGIC 777

 #define __QLCNIC_FW_ATTACHED           0
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
index cec0908..1a5915d 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c
@@ -1408,23 +1408,21 @@ void qlcnic_82xx_process_rcv_ring_diag(struct qlcnic_host_sds_ring *sds_ring)
 int qlcnic_82xx_napi_add(struct qlcnic_adapter *adapter,
                         struct net_device *netdev)
 {
-       int ring, max_sds_rings;
+       int ring;
        struct qlcnic_host_sds_ring *sds_ring;
        struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;

        if (qlcnic_alloc_sds_rings(recv_ctx, adapter->max_sds_rings))
                return -ENOMEM;

-       max_sds_rings = adapter->max_sds_rings;
-
        for (ring = 0; ring < adapter->max_sds_rings; ring++) {
                sds_ring = &recv_ctx->sds_rings[ring];
                if (ring == adapter->max_sds_rings - 1)
                        netif_napi_add(netdev, &sds_ring->napi, qlcnic_poll,
-                                      QLCNIC_NETDEV_WEIGHT / max_sds_rings);
+                                      NAPI_POLL_WEIGHT);
                else
                        netif_napi_add(netdev, &sds_ring->napi, qlcnic_rx_poll,
-                                      QLCNIC_NETDEV_WEIGHT*2);
+                                      NAPI_POLL_WEIGHT);
        }

        if (qlcnic_alloc_tx_rings(adapter, netdev)) {
@@ -1861,7 +1859,7 @@ void qlcnic_83xx_napi_disable(struct qlcnic_adapter *adapter)
 int qlcnic_83xx_napi_add(struct qlcnic_adapter *adapter,
                         struct net_device *netdev)
 {
-       int ring, max_sds_rings, temp;
+       int ring;
        struct qlcnic_host_sds_ring *sds_ring;
        struct qlcnic_host_tx_ring *tx_ring;
        struct qlcnic_recv_context *recv_ctx = adapter->recv_ctx;
@@ -1869,25 +1867,22 @@ int qlcnic_83xx_napi_add(struct qlcnic_adapter *adapter,
        if (qlcnic_alloc_sds_rings(recv_ctx, adapter->max_sds_rings))
                return -ENOMEM;

-       max_sds_rings = adapter->max_sds_rings;
        for (ring = 0; ring < adapter->max_sds_rings; ring++) {
                sds_ring = &recv_ctx->sds_rings[ring];
                if (adapter->flags & QLCNIC_MSIX_ENABLED) {
-                       if (!(adapter->flags & QLCNIC_TX_INTR_SHARED)) {
+                       if (!(adapter->flags & QLCNIC_TX_INTR_SHARED))
                                netif_napi_add(netdev, &sds_ring->napi,
                                               qlcnic_83xx_rx_poll,
-                                              QLCNIC_NETDEV_WEIGHT * 2);
-                       } else {
-                               temp = QLCNIC_NETDEV_WEIGHT / max_sds_rings;
+                                              NAPI_POLL_WEIGHT);
+                       else
                                netif_napi_add(netdev, &sds_ring->napi,
                                               qlcnic_83xx_msix_sriov_vf_poll,
-                                              temp);
-                       }
+                                              NAPI_POLL_WEIGHT);

                } else {
                        netif_napi_add(netdev, &sds_ring->napi,
                                       qlcnic_83xx_poll,
-                                      QLCNIC_NETDEV_WEIGHT / max_sds_rings);
+                                      NAPI_POLL_WEIGHT);
                }
        }

@@ -1902,7 +1897,7 @@ int qlcnic_83xx_napi_add(struct qlcnic_adapter *adapter,
                        tx_ring = &adapter->tx_ring[ring];
                        netif_napi_add(netdev, &tx_ring->napi,
                                       qlcnic_83xx_msix_tx_poll,
-                                      QLCNIC_NETDEV_WEIGHT);
+                                      NAPI_POLL_WEIGHT);
                }
        }

We are in process of running performance numbers with these changes.  I will respond once I have the performance data with the proposed changes.

Thanks,
Himanshu
--
1.8.3.1

^ permalink raw reply related

* Re: [PATCH 0/2] fix generic netlink locking issue(s)
From: Johannes Berg @ 2013-08-22  6:51 UTC (permalink / raw)
  To: Pravin Shelar; +Cc: Oliver Hartkopp, netdev, Thomas Graf
In-Reply-To: <CALnjE+qtGXHkqxfSPWi5nsqA=fLkxijxDLDha=Z+YL=eQxxDmQ@mail.gmail.com>

On Wed, 2013-08-21 at 15:53 -0700, Pravin Shelar wrote:
> This looks like more general problem and not specific to genl_ctrl
> family. I have couple of fixes for issues you are seeing, I will send
> out those patches soon.

No, it's not a general problem.

You don't _want_ dump generally locked, it's not really useful. Netlink
guarantees with the cb_mutex that it can't go away, so locking dump at
the generic netlink level doesn't buy the average generic netlink user
at all.

I think that the problem is that generic netlink is "inside itself" in a
manner of speaking.

OTOH, I'm not sure I fully understand what would happen if a family is
deleted while something inside of it is dumping. Even the module unload
wouldn't be prevented since netlink_dump_start() uses THIS_MODULE (which
is really NULL since generic netlink is built into the kernel), so maybe
you're right about the locking, but then your patch still wouldn't be
correct because the family might still go away even if you lock the
genl_lock() every round.

I think to fix that additional problem we need something like the
(untested!) patch below (which is missing documentation updates) in
addition to fix that problem.

johannes

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 93024a4..0bc3a53 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -42,6 +42,7 @@ struct genl_info;
  * @ops_list: list of all assigned operations
  * @family_list: family list
  * @mcast_groups: multicast groups list
+ * @module: the module containing this family
  */
 struct genl_family {
 	unsigned int		id;
@@ -61,6 +62,7 @@ struct genl_family {
 	struct list_head	ops_list;	/* private */
 	struct list_head	family_list;	/* private */
 	struct list_head	mcast_groups;	/* private */
+	struct module *		module;		/* private */
 };
 
 /**
@@ -121,9 +123,24 @@ struct genl_ops {
 	struct list_head	ops_list;
 };
 
-extern int genl_register_family(struct genl_family *family);
-extern int genl_register_family_with_ops(struct genl_family *family,
+extern int __genl_register_family(struct genl_family *family);
+extern int __genl_register_family_with_ops(struct genl_family *family,
 	struct genl_ops *ops, size_t n_ops);
+
+static inline int genl_register_family(struct genl_family *family)
+{
+	family->module = THIS_MODULE;
+	return __genl_register_family(family);
+}
+
+static inline int genl_register_family_with_ops(struct genl_family *family,
+						struct genl_ops *ops,
+						size_t n_ops)
+{
+	family->module = THIS_MODULE;
+	return __genl_register_family_with_ops(family, ops, n_ops);
+}
+
 extern int genl_unregister_family(struct genl_family *family);
 extern int genl_register_ops(struct genl_family *, struct genl_ops *ops);
 extern int genl_unregister_ops(struct genl_family *, struct genl_ops *ops);
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 2fd6dbe..8bab493 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -374,7 +374,7 @@ EXPORT_SYMBOL(genl_unregister_ops);
  *
  * Return 0 on success or a negative error code.
  */
-int genl_register_family(struct genl_family *family)
+int __genl_register_family(struct genl_family *family)
 {
 	int err = -EINVAL;
 
@@ -430,7 +430,7 @@ errout_locked:
 errout:
 	return err;
 }
-EXPORT_SYMBOL(genl_register_family);
+EXPORT_SYMBOL(__genl_register_family);
 
 /**
  * genl_register_family_with_ops - register a generic netlink family
@@ -457,12 +457,12 @@ EXPORT_SYMBOL(genl_register_family);
  *
  * Return 0 on success or a negative error code.
  */
-int genl_register_family_with_ops(struct genl_family *family,
+int __genl_register_family_with_ops(struct genl_family *family,
 	struct genl_ops *ops, size_t n_ops)
 {
 	int err, i;
 
-	err = genl_register_family(family);
+	err = __genl_register_family(family);
 	if (err)
 		return err;
 
@@ -476,7 +476,7 @@ err_out:
 	genl_unregister_family(family);
 	return err;
 }
-EXPORT_SYMBOL(genl_register_family_with_ops);
+EXPORT_SYMBOL(__genl_register_family_with_ops);
 
 /**
  * genl_unregister_family - unregister generic netlink family

^ permalink raw reply related

* [PATCH net] be2net: fix disabling TX in be_close()
From: Sathya Perla @ 2013-08-22  6:53 UTC (permalink / raw)
  To: netdev

commit fba875591 ("disable TX in be_close()") disabled TX in be_close()
to protect be_xmit() from touching freed up queues in the AER recovery
flow.  But, TX must be disabled *before* cleaning up TX completions in
the close() path, not after. This allows be_tx_compl_clean() to free up
all TX-req skbs that were notified to the HW.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
Re-sending the patch with summary line of the commit referenced in
the change log.

 drivers/net/ethernet/emulex/benet/be_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 181edb5..4559c35 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2563,8 +2563,8 @@ static int be_close(struct net_device *netdev)
 	/* Wait for all pending tx completions to arrive so that
 	 * all tx skbs are freed.
 	 */
-	be_tx_compl_clean(adapter);
 	netif_tx_disable(netdev);
+	be_tx_compl_clean(adapter);
 
 	be_rx_qs_destroy(adapter);
 
-- 
1.7.1

^ permalink raw reply related

* Re: ipvsadm: One-packet scheduling with UDP service is unstable
From: Julian Anastasov @ 2013-08-22  6:43 UTC (permalink / raw)
  To: Drunkard Zhang
  Cc: Wensong Zhang, Simon Horman, Pablo Neira Ayuso, Patrick McHardy,
	Jozsef Kadlecsik, David S. Miller, netdev, lvs-devel,
	netfilter-devel, netfilter, coreteam, linux-kernel
In-Reply-To: <CAJiejCQPa7V703RwOeWPpbQMJQvfswRn+pvTKFmNdeU05RU3SQ@mail.gmail.com>


	Hello,

On Tue, 20 Aug 2013, Drunkard Zhang wrote:

> Need help here, thank you for replying :-)
> 
> I'm setting up a syslog cluster based on IPVS, all UDP datagrams sent
> from firewall with fixed source IP and fixed source port, so
> pseudo-random balancing based on client IP and port won't working. And
> it seems that keepalived is not supporting One-packet scheduling
> option, so I did some hacks on it after keepalived started:
> 
> 1. dump LVS rules with ipvsadm -S -n > rules-vs3;
> 2. add --ops option;
> 3. restore LVS rules with ipvsadm-restore < rules-vs3;
> 4. dump the running LVS rules with ipvsadm -S -n
> 
> So, I got two problems here:
> 
> 1. Dumped rules in step 4 above is not usable anymore, the double-dash
> in --ops lost, so I can't restore rule with this dump anymore. This
> must be a bug.
> 
> 2. The --ops option is not working sometimes you applied the rules,
> and in most of times the --ops just not working. To make it work, just
> 'ipvsadm-restore < rules-vs3' for plenty of times until it's working.
> I haven't find the patterns make it work yet. This is lucky, I can't
> get it work on second host at all.
> 
> The "not working" above means the UDP datagrams from one source IP is
> sticked to one realserver, it doesn't distribute to other realservers
> which --ops designed for.

	Can you try with recent ipvsadm from git:

git clone git://git.kernel.org/pub/scm/utils/kernel/ipvsadm/ipvsadm.git

	I see related commit that will print -o for
the OPS feature:

===
commit 6a03100c189d00e3a8235215392465b5b877ba8f
Author: Krzysztof Gajdemski <songo@debian.org.pl>
Date:   Thu Mar 21 11:40:06 2013 +0100

    ipvsadm: Fix wrong format of -o option in FMT_RULE listing

    'ipvsadm -S' listed one-packet scheduling option in wrong format
    ('ops' instead of '--ops' or '-o') preventing any service with OPS
    feature from restoring using 'ipvsadm -R'.  Now we use '-o' which
    works well with save/restore commands.

    Signed-off-by: Krzysztof Gajdemski <songo@debian.org.pl>
    Signed-off-by: Simon Horman <horms@verge.net.au>
===

	Let me know if you still have any problems with OPS.
Sending to lvs-devel@vger.kernel.org and
lvs-users@linuxvirtualserver.org should be enough for
ipvsadm related discussions.

> So I wondering if there's some CONFIG_* options that ipvs needs, or
> recent development broke the code?

	No kernel options should be related to OPS. I assume
you are not using the SH scheduler. Make sure the OPS mode
is properly applied to the virtual service, check for "ops"
in the configuration:

cat /proc/net/ip_vs

> Do I need to attach kernel config file?

	No

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [RFC] phy: micrel: Convert micrel PHY driver to use OF
From: Sascha Hauer @ 2013-08-22  6:34 UTC (permalink / raw)
  To: Olof Johansson
  Cc: dinguyen, dinh.linux, netdev, Richard Cochran, Linus Walleij,
	Felipe Balbi, David S. Miller, Giuseppe Cavallaro, Rob Herring,
	devicetree
In-Reply-To: <20130822051347.GD5151@quad.lixom.net>

On Wed, Aug 21, 2013 at 10:13:47PM -0700, Olof Johansson wrote:
> (adding devicetree@vger.kernel.org)
> 
> 
> On Tue, Aug 13, 2013 at 11:42:36AM -0500, dinguyen@altera.com wrote:
> > ---
> > Hello,
> > 
> > I would like to solicit comments on the need to convert the ethernet PHY
> > drivers to use OF/device trees? For the platform that I'm interested in,
> > SOCFPGA, it is using the stmicro ethernet driver. It has a Micrel PHY
> > on the board. The only way that I know of how to change the skew settings
> > for the phy is through a board level initialization.
> > 
> > One of the ARM maintainers suggested that perhaps refactoring the ethernet
> > driver to use device tree would be nice. But that would not help me with
> > configuring the PHY settings.
> > 
> > So a little investigation led me to believe that refactoring the /net/phy
> > drivers into a device tree implementation would help greatly. I was thinking
> > it could be done like the pinctrl or some of the usb/phy driver.
> > 
> > Since I am only familiar with the ARM SoC space, I want to make sure that
> > this idea is right approach. I can start with the micrel PHY driver
> > first, as that is the only HW I have access to.
> 
> Hi,
> 
> Sorry for the slow reply here.
> 
> I don't think this is quite the right approach.
> 
> What you want to do is to make the phy devices register based on device tree
> contents, which also means removing the run function, or rather moving it to
> a generic run function in the phy subsystem that acts based on device tree
> contents instead of a hard-coded per-board run function.
> 
> It sounds like defining that binding might end up getting complicated.
> I suggest you consider recruiting some of the more seasoned devicetree folks on
> this endeavor.
> 
> It's possible that you'll mostly have per-vendor/phy type properties to tune
> the various settings, but it's also likely that you will have some generic and
> shared (optional) properties such as gpios for resetting, or regulators for
> powering, the phy.

This patch recently was merged:

http://patchwork.ozlabs.org/patch/268661/

It solves exactly the same problem of specifying the skew settings.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ 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