Netdev List
 help / color / mirror / Atom feed
* [PATCH 00/17 v5] usb/net: rndis: first step toward consolidation
From: Linus Walleij @ 2012-05-12  8:15 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	David S. Miller, Greg Kroah-Hartman, Felipe Balbi
  Cc: Jussi Kivilinna, Haiyang Zhang, Wei Yongjun, Ben Hutchings,
	Linus Walleij

A fifth iteration of the first steps of RNDIS consolidation.

This version mainly address byte order issues found with
sparse by Jussi Kivilinna, his fixes are folded into patch 1.
At the end I have added Jussis patches cleaning up rndis_host and
rndis_wlan based on top of this series.

Jussi Kivilinna (4):
  rndis_host: cleanup: byteswap data from device instead of RNDIS_*
    defines
  rndis_wlan: cleanup: byteswap data from device instead of RNDIS_*
    defines
  rndis_host: cleanup: change oid from __le32 to u32 in rndis_query()
  rndis_wlan: cleanup: change oid from __le32 to u32 in various places

Linus Walleij (13):
  usb/net: rndis: inline the cpu_to_le32() macro
  usb/net: rndis: break out <linux/rndis.h> defines
  usb/net: rndis: remove ambigous status codes
  usb/net: rndis: eliminate first set of duplicate OIDs
  usb/net: rndis: merge duplicate 802_* OIDs
  usb/net: rndis: delete surplus defines
  usb/net: rndis: group all status codes together
  usb/net: rndis: merge media type definitions
  usb/net: rndis: delete duplicate packet types
  usb/net: rndis: move and namespace PnP defines
  usb/net: rndis: merge command codes
  usb/net: rndis: fixup a few name prefixes
  usb/net: rndis: move bus message definition

 drivers/net/hyperv/hyperv_net.h   |  290 +---------------------------
 drivers/net/hyperv/rndis_filter.c |   46 +++---
 drivers/net/usb/rndis_host.c      |   83 ++++----
 drivers/net/wireless/rndis_wlan.c |  361 +++++++++++++++++------------------
 drivers/usb/gadget/f_rndis.c      |    6 +-
 drivers/usb/gadget/ndis.h         |  164 ----------------
 drivers/usb/gadget/rndis.c        |  271 +++++++++++++-------------
 drivers/usb/gadget/rndis.h        |   48 +-----
 include/linux/rndis.h             |  390 +++++++++++++++++++++++++++++++++++++
 include/linux/usb/rndis_host.h    |   66 +------
 10 files changed, 774 insertions(+), 951 deletions(-)
 create mode 100644 include/linux/rndis.h

-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH RFC 1/6] skbuff: support per-page destructors in copy_ubufs
From: Ian Campbell @ 2012-05-12  6:01 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David Miller, netdev@vger.kernel.org, eric.dumazet@gmail.com
In-Reply-To: <20120511163057.GB10344@redhat.com>

On Fri, 2012-05-11 at 17:30 +0100, Michael S. Tsirkin wrote:
> On Fri, May 11, 2012 at 03:08:36PM +0300, Michael S. Tsirkin wrote:
> > On Fri, May 11, 2012 at 11:58:12AM +0100, Ian Campbell wrote:
> > > On Fri, 2012-05-11 at 10:00 +0100, Ian Campbell wrote:
> > > > I'm seeing copy_ubufs called in my remote NFS test, which I don't
> > > > think I expected -- I'll investigate why this is happening today. 
> > > 
> > > It's tcp_transmit_skb which can (conditionally) call skb_clone
> > > (backtrace below)
> > 
> > Interesting. I didn't realise we clone skbs on data path:
> > tcp_write_xmit calls tcp_transmit_skb with clone_it flag.
> > Could someone comment on why we need to clone on good path
> > like this?
> 
> Hmm, it's in case we need to retransmit it later.

I wonder if we could avoid the copy_ubuf in this particular clone path
and have any subsequent calls to copy_ubufs use skb->fclone to determine
if it can safely replace the frags?

If it cannot then could it do a full copy of the skb (including new
shinfo, new frag pages etc) as a fallback?

Ian.

^ permalink raw reply

* Re: [net-next 06/12] ixgbe: Hardware Timestamping + PTP Hardware Clock (PHC)
From: Richard Cochran @ 2012-05-12  5:34 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <02874ECE860811409154E81DA85FBB58077949E8@ORSMSX105.amr.corp.intel.com>

On Fri, May 11, 2012 at 07:23:44PM +0000, Keller, Jacob E wrote:
> 
>
> I believe this very rare case might be possible, but I don't think
> that checking the ptp seqid will fix anything. In normal cases,
> hardware latches Rx packet timestamp, then the ptp packet goes into
> the queue and we process it shortly after. Before we process that
> packet there will never be another packet in the queue that needs a
> timestamp. We know this because the hardware stops timestamping
> until we unlatch the RX registers. This should mean we don't need to
> check the sequence ID, and spending time doing it would never fix
> the issue you are talking about.
>
> The issue is for when a packet is timestamped and then never reaches
> the queue. Then the rx stamp registers are locked for good, because
> we never clear them, and hardware would never timestamp another
> receive packet. I don't know a good solution to this, except to
> clear the registers periodically. Do you have any suggestions?

Well, one solution would be to check every received packet with the
BPF in ptp_classify.h (whenever Rx time stamping is enabled).

When the driver finds an event packet in the Rx queue, and
TSYNCRXCTL[RXTT] is set, it reads out the time stamp along with
RXSATRL/H. If the fields match, then add the time stamp to the skb.

[ Or perhaps instead of using RXSATRL/H, just use the descriptor bit.
  If *not* set, then the time stamp does not belong to this packet. ]

HTH,
Richard

^ permalink raw reply

* Re: [net-next 06/12] ixgbe: Hardware Timestamping + PTP Hardware Clock (PHC)
From: Richard Cochran @ 2012-05-12  5:24 UTC (permalink / raw)
  To: Keller, Jacob E
  Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
	gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <02874ECE860811409154E81DA85FBB5807794A06@ORSMSX105.amr.corp.intel.com>

On Fri, May 11, 2012 at 07:34:12PM +0000, Keller, Jacob E wrote:
> 
> Sorry for the spam here, but I looked at the ixgbe code, and found a
> solution. When ptp4l discovers a missing rx timestamp, it faults and
> then waits for 15 seconds until the fault is cleared. After this, it
> reopens the socket, and reruns the hwtstamp ioctl. This function
> actually does clear the tx/rx timestamps (just in case) So after a
> fault the values should end up being reset. Is this good enough?

Yes, for ptp4l, that will work, but this program is acting extremely
defensively in that situation. Not every user space program will
necessarily behave in the same way. The classic ptpd just ignores
packets with missing time stamps, IRRC.

Richard

^ permalink raw reply

* [PATCH net-next v3] be2net: Fix to allow get/set of debug levels in the firmware.
From: Somnath Kotur @ 2012-05-12  3:39 UTC (permalink / raw)
  To: netdev, bhutchings; +Cc: Somnath Kotur, Suresh Redy

Pls ignore previous patch v2. 
Removed unwanted brace.
Incorporated comments from Ben Hutchings.

Signed-off-by: Suresh Redy <suresh.reddy@emulex.com>
Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be.h         |    3 +
 drivers/net/ethernet/emulex/benet/be_cmds.c    |   56 +++++++++++++++++
 drivers/net/ethernet/emulex/benet/be_cmds.h    |   57 +++++++++++++++++
 drivers/net/ethernet/emulex/benet/be_ethtool.c |   77 ++++++++++++++++++++++++
 drivers/net/ethernet/emulex/benet/be_main.c    |   37 +++++++++++
 5 files changed, 230 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index c3ee910..9817fed 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -415,6 +415,7 @@ struct be_adapter {
 	bool wol;
 	u32 max_pmac_cnt;	/* Max secondary UC MACs programmable */
 	u32 uc_macs;		/* Count of secondary UC MAC programmed */
+	u32 msg_enable;
 };
 
 #define be_physfn(adapter) (!adapter->is_virtfn)
@@ -603,4 +604,6 @@ extern void be_parse_stats(struct be_adapter *adapter);
 extern int be_load_fw(struct be_adapter *adapter, u8 *func);
 extern bool be_is_wol_supported(struct be_adapter *adapter);
 extern bool be_pause_supported(struct be_adapter *adapter);
+extern u32 be_get_fw_log_level(struct be_adapter *adapter);
+
 #endif				/* BE_H */
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 43167e8..b24623c 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -2589,4 +2589,60 @@ err:
 	mutex_unlock(&adapter->mbox_lock);
 	pci_free_consistent(adapter->pdev, cmd.size, cmd.va, cmd.dma);
 	return status;
+
+}
+int be_cmd_get_ext_fat_capabilites(struct be_adapter *adapter,
+				   struct be_dma_mem *cmd)
+{
+	struct be_mcc_wrb *wrb;
+	struct be_cmd_req_get_ext_fat_caps *req;
+	int status;
+
+	if (mutex_lock_interruptible(&adapter->mbox_lock))
+		return -1;
+
+	wrb = wrb_from_mbox(adapter);
+	if (!wrb) {
+		status = -EBUSY;
+		goto err;
+	}
+
+	req = cmd->va;
+	be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
+			       OPCODE_COMMON_GET_EXT_FAT_CAPABILITES,
+			       cmd->size, wrb, cmd);
+	req->parameter_type = cpu_to_le32(1);
+
+	status = be_mbox_notify_wait(adapter);
+err:
+	mutex_unlock(&adapter->mbox_lock);
+	return status;
+}
+
+int be_cmd_set_ext_fat_capabilites(struct be_adapter *adapter,
+				   struct be_dma_mem *cmd,
+				   struct be_fat_conf_params *configs)
+{
+	struct be_mcc_wrb *wrb;
+	struct be_cmd_req_set_ext_fat_caps *req;
+	int status;
+
+	spin_lock_bh(&adapter->mcc_lock);
+
+	wrb = wrb_from_mccq(adapter);
+	if (!wrb) {
+		status = -EBUSY;
+		goto err;
+	}
+
+	req = cmd->va;
+	memcpy(&req->set_params, configs, sizeof(struct be_fat_conf_params));
+	be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
+			       OPCODE_COMMON_SET_EXT_FAT_CAPABILITES,
+			       cmd->size, wrb, cmd);
+
+	status = be_mcc_notify_wait(adapter);
+err:
+	spin_unlock_bh(&adapter->mcc_lock);
+	return status;
 }
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 944f031..0b1029b 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -189,6 +189,8 @@ struct be_mcc_mailbox {
 #define OPCODE_COMMON_GET_PHY_DETAILS			102
 #define OPCODE_COMMON_SET_DRIVER_FUNCTION_CAP		103
 #define OPCODE_COMMON_GET_CNTL_ADDITIONAL_ATTRIBUTES	121
+#define OPCODE_COMMON_GET_EXT_FAT_CAPABILITES		125
+#define OPCODE_COMMON_SET_EXT_FAT_CAPABILITES		126
 #define OPCODE_COMMON_GET_MAC_LIST			147
 #define OPCODE_COMMON_SET_MAC_LIST			148
 #define OPCODE_COMMON_GET_HSW_CONFIG			152
@@ -1602,6 +1604,56 @@ static inline void *be_erx_stats_from_cmd(struct be_adapter *adapter)
 	}
 }
 
+
+/************** get fat capabilites *******************/
+#define MAX_MODULES 27
+#define MAX_MODES 4
+#define MODE_UART 0
+#define FW_LOG_LEVEL_DEFAULT 48
+#define FW_LOG_LEVEL_FATAL 64
+
+struct ext_fat_mode {
+	u8 mode;
+	u8 rsvd0;
+	u16 port_mask;
+	u32 dbg_lvl;
+	u64 fun_mask;
+} __packed;
+
+struct ext_fat_modules {
+	u8 modules_str[32];
+	u32 modules_id;
+	u32 num_modes;
+	struct ext_fat_mode trace_lvl[MAX_MODES];
+} __packed;
+
+struct be_fat_conf_params {
+	u32 max_log_entries;
+	u32 log_entry_size;
+	u8 log_type;
+	u8 max_log_funs;
+	u8 max_log_ports;
+	u8 rsvd0;
+	u32 supp_modes;
+	u32 num_modules;
+	struct ext_fat_modules module[MAX_MODULES];
+} __packed;
+
+struct be_cmd_req_get_ext_fat_caps {
+	struct be_cmd_req_hdr hdr;
+	u32 parameter_type;
+};
+
+struct be_cmd_resp_get_ext_fat_caps {
+	struct be_cmd_resp_hdr hdr;
+	struct be_fat_conf_params get_params;
+};
+
+struct be_cmd_req_set_ext_fat_caps {
+	struct be_cmd_req_hdr hdr;
+	struct be_fat_conf_params set_params;
+};
+
 extern int be_pci_fnum_get(struct be_adapter *adapter);
 extern int be_cmd_POST(struct be_adapter *adapter);
 extern int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
@@ -1707,4 +1759,9 @@ extern int be_cmd_set_hsw_config(struct be_adapter *adapter, u16 pvid,
 extern int be_cmd_get_hsw_config(struct be_adapter *adapter, u16 *pvid,
 			u32 domain, u16 intf_id);
 extern int be_cmd_get_acpi_wol_cap(struct be_adapter *adapter);
+extern int be_cmd_get_ext_fat_capabilites(struct be_adapter *adapter,
+					  struct be_dma_mem *cmd);
+extern int be_cmd_set_ext_fat_capabilites(struct be_adapter *adapter,
+					  struct be_dma_mem *cmd,
+					  struct be_fat_conf_params *cfgs);
 
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 747f68f..2b5164f 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -878,6 +878,81 @@ be_read_eeprom(struct net_device *netdev, struct ethtool_eeprom *eeprom,
 	return status;
 }
 
+static u32 be_get_msg_level(struct net_device *netdev)
+{
+	struct be_adapter *adapter = netdev_priv(netdev);
+
+	if (lancer_chip(adapter)) {
+		dev_err(&adapter->pdev->dev, "Operation not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	return adapter->msg_enable;
+}
+
+static void be_set_fw_log_level(struct be_adapter *adapter, u32 level)
+{
+	struct be_dma_mem extfat_cmd;
+	struct be_fat_conf_params *cfgs;
+	int status;
+	int i, j;
+
+	memset(&extfat_cmd, 0, sizeof(struct be_dma_mem));
+	extfat_cmd.size = sizeof(struct be_cmd_resp_get_ext_fat_caps);
+	extfat_cmd.va = pci_alloc_consistent(adapter->pdev, extfat_cmd.size,
+					     &extfat_cmd.dma);
+	if (!extfat_cmd.va) {
+		dev_err(&adapter->pdev->dev, "%s: Memory allocation failure\n",
+			__func__);
+		goto err;
+	}
+	status = be_cmd_get_ext_fat_capabilites(adapter, &extfat_cmd);
+	if (!status) {
+		cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
+					sizeof(struct be_cmd_resp_hdr));
+		for (i = 0; i < cfgs->num_modules; i++) {
+			for (j = 0; j < cfgs->module[i].num_modes; j++) {
+				if (cfgs->module[i].trace_lvl[j].mode ==
+								MODE_UART)
+					cfgs->module[i].trace_lvl[j].dbg_lvl =
+							cpu_to_le32(level);
+			}
+		}
+		status = be_cmd_set_ext_fat_capabilites(adapter, &extfat_cmd,
+							cfgs);
+		if (status)
+			dev_err(&adapter->pdev->dev,
+				"Message level set failed\n");
+	} else {
+		dev_err(&adapter->pdev->dev, "Message level get failed\n");
+	}
+
+	pci_free_consistent(adapter->pdev, extfat_cmd.size, extfat_cmd.va,
+			    extfat_cmd.dma);
+err:
+	return;
+}
+
+static void be_set_msg_level(struct net_device *netdev, u32 level)
+{
+	struct be_adapter *adapter = netdev_priv(netdev);
+
+	if (lancer_chip(adapter)) {
+		dev_err(&adapter->pdev->dev, "Operation not supported\n");
+		return;
+	}
+
+	if (adapter->msg_enable == level)
+		return;
+
+	if (level & NETIF_MSG_HW != adapter->msg_enable & NETIF_MSG_HW)
+		be_set_fw_log_level(adapter, level & NETIF_MSG_HW ?
+			       	    FW_LOG_LEVEL_DEFAULT : FW_LOG_LEVEL_FATAL);
+	adapter->msg_enable = level;
+
+	return;
+}
+
 const struct ethtool_ops be_ethtool_ops = {
 	.get_settings = be_get_settings,
 	.get_drvinfo = be_get_drvinfo,
@@ -893,6 +968,8 @@ const struct ethtool_ops be_ethtool_ops = {
 	.set_pauseparam = be_set_pauseparam,
 	.get_strings = be_get_stat_strings,
 	.set_phys_id = be_set_phys_id,
+	.get_msglevel = be_get_msg_level,
+	.set_msglevel = be_set_msg_level,
 	.get_sset_count = be_get_sset_count,
 	.get_ethtool_stats = be_get_ethtool_stats,
 	.get_regs_len = be_get_reg_len,
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 6d5d30b..c2286a2 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3367,9 +3367,43 @@ bool be_is_wol_supported(struct be_adapter *adapter)
 		!be_is_wol_excluded(adapter)) ? true : false;
 }
 
+u32 be_get_fw_log_level(struct be_adapter *adapter)
+{
+	struct be_dma_mem extfat_cmd;
+	struct be_fat_conf_params *cfgs;
+	int status;
+	u32 level = 0;
+	int j;
+
+	memset(&extfat_cmd, 0, sizeof(struct be_dma_mem));
+	extfat_cmd.size = sizeof(struct be_cmd_resp_get_ext_fat_caps);
+	extfat_cmd.va = pci_alloc_consistent(adapter->pdev, extfat_cmd.size,
+					     &extfat_cmd.dma);
+	if (!extfat_cmd.va) {
+		dev_err(&adapter->pdev->dev, "%s: Memory allocation failure\n",
+			__func__);
+		goto err;
+	}
+
+	status = be_cmd_get_ext_fat_capabilites(adapter, &extfat_cmd);
+	if (!status) {
+		cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
+						sizeof(struct be_cmd_resp_hdr));
+		for (j = 0; j < 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;
+		}
+	}
+	pci_free_consistent(adapter->pdev, extfat_cmd.size, extfat_cmd.va,
+			    extfat_cmd.dma);
+err:
+	return level;
+}
+
 static int be_get_config(struct be_adapter *adapter)
 {
 	int status;
+	u32 level;
 
 	status = be_cmd_query_fw_cfg(adapter, &adapter->port_num,
 			&adapter->function_mode, &adapter->function_caps);
@@ -3407,6 +3441,9 @@ static int be_get_config(struct be_adapter *adapter)
 	if (be_is_wol_supported(adapter))
 		adapter->wol = true;
 
+	level = be_get_fw_log_level(adapter);
+	adapter->msg_enable = level <= FW_LOG_LEVEL_DEFAULT ? NETIF_MSG_HW : 0;
+
 	return 0;
 }
 
-- 
1.5.6.1

^ permalink raw reply related

* Re: [3.2.y] phy:icplus:fix Auto Power Saving in ip101a_config_init.
From: Ben Hutchings @ 2012-05-12  0:57 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: stable, David Miller, srinivas.kandagatla, netdev,
	peppe.cavallaro
In-Reply-To: <20120509171847.GC4884@burratino>

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

On Wed, 2012-05-09 at 12:18 -0500, Jonathan Nieder wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> Date: Mon, 2 Apr 2012 00:02:09 +0000
> 
> [ Upstream commit b3300146aa8efc5d3937fd33f3cfdc580a3843bc ]
[...]
> David Miller wrote:
> > From: Jonathan Nieder <jrnieder@gmail.com>
> 
> >> I'm guessing 3.2.y needs this, too, since it also includes
> >> v3.2-rc1~129^2~247 ("net/phy: add IC+ IP101A and support APS").
> >>
> >> Here's a quick backport made by adjusting the context line to
> >> use IP101A_APS_ON instead of IP101A_G_APS_ON.
> >>
> >> Please include it in some later stable update if appropriate.
> >
> > Please submit this directly to the stable list.
> 
> Thanks for looking it over.  Doing so.
[...]

I've queued this up, thanks.

Ben.

-- 
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
                                                         - Carolyn Scheppner

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

^ permalink raw reply

* Re: [PATCH] net: of/phy: fix build error when phylib is built as a module
From: Randy Dunlap @ 2012-05-11 23:50 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: David Miller, paul.gortmaker, sfr, linux-next, linux-kernel,
	netdev, linuxppc-dev, david.daney
In-Reply-To: <1336751221-19127-1-git-send-email-bjorn@mork.no>

On 05/11/2012 08:47 AM, Bjørn Mork wrote:

> CONFIG_OF_MDIO is tristate and will be m if PHYLIB is m.  Use
> IS_ENABLED macro to prevent build error:
> 
>  ERROR: "of_mdio_find_bus" [drivers/net/phy/mdio-mux.ko] undefined!
> 
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
> Cc: David Daney <david.daney@cavium.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>


Acked-by: Randy Dunlap <rdunlap@xenotime.net>

Thanks.


> ---
> I wonder if this could be as banal as this?  Not even build tested...
> 
> Should be wrapped into commit 25106022 if it works, to ensure
> bisectability.
> 
> 
> 
> Bjørn
> 
>  drivers/net/phy/mdio_bus.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 83d5c9f..683ef1c 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -88,7 +88,7 @@ static struct class mdio_bus_class = {
>  	.dev_release	= mdiobus_release,
>  };
>  
> -#ifdef CONFIG_OF_MDIO
> +#if IS_ENABLED(CONFIG_OF_MDIO)
>  /* Helper function for of_mdio_find_bus */
>  static int of_mdio_bus_match(struct device *dev, void *mdio_bus_np)
>  {



-- 
~Randy

^ permalink raw reply

* Re: [PATCH 0/2] Add and use vsprintf extension %pMbt for bluetooth macs
From: Joe Perches @ 2012-05-11 23:21 UTC (permalink / raw)
  To: Andrei Emeltchenko
  Cc: Marcel Holtmann, Gustavo F. Padovan, linux-bluetooth, netdev,
	linux-kernel
In-Reply-To: <20120509090156.GB8473@aemeltch-MOBL1>

On Wed, 2012-05-09 at 12:01 +0300, Andrei Emeltchenko wrote:
> Hi Joe
> 
> On Fri, Dec 03, 2010 at 06:33:02PM -0800, Joe Perches wrote:
> > Using vsprintf extensions can save text and data.
> > Add %pMbt for the byte reversed output for bluetooth addresses.
> > 
> > Joe Perches (2):
> >   vsprintf: Add %pMbt, bluetooth mac address
> >   bluetooth: Use printf extension %pMbt
> 
> I think this would be the best way to solve our issues with batostr.
> 
> BTW: What is the status with this patch series?

18 month old patches generally don't apply.

If you want to bring it forward or redo it,
please do.

^ permalink raw reply

* RE: [net-next 2/2] igb: Add Support for new i210/i211 devices.
From: Wyborny, Carolyn @ 2012-05-11 23:02 UTC (permalink / raw)
  To: David Miller, Kirsher, Jeffrey T
  Cc: netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <20120511.182735.63933163877487008.davem@davemloft.net>

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Friday, May 11, 2012 3:28 PM
>To: Kirsher, Jeffrey T
>Cc: Wyborny, Carolyn; netdev@vger.kernel.org; gospo@redhat.com;
>sassmann@redhat.com
>Subject: Re: [net-next 2/2] igb: Add Support for new i210/i211 devices.
>
>From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>Date: Fri, 11 May 2012 01:01:22 -0700
>
>> +s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
>> +{
>> +	s32 ret_val;
>> +
>> +	ret_val = igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
>> +
>> +	return ret_val;
>> +}
>
>Please, this is just unnecessary syntactic masterbation, simplify this
>to:
>
>s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
>{
>	return igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
>}
>
>And:
>
>> +void igb_release_nvm_i210(struct e1000_hw *hw)
>> +{
>> +
>> +	igb_release_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
>> +}
>
>Please get rid of that unnecessary empty line.
>
>I can really tell when someone is extremely careless when removing
>their debugging code, and I can almost guarentee that's what has
>happened here.

Yes, you're right.  Will fix ASAP.

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

^ permalink raw reply

* Re: [net-next 2/2] igb: Add Support for new i210/i211 devices.
From: Jeff Kirsher @ 2012-05-11 23:02 UTC (permalink / raw)
  To: David Miller; +Cc: carolyn.wyborny, netdev, gospo, sassmann
In-Reply-To: <20120511.182735.63933163877487008.davem@davemloft.net>

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

On Fri, 2012-05-11 at 18:27 -0400, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Fri, 11 May 2012 01:01:22 -0700
> 
> > +s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
> > +{
> > +	s32 ret_val;
> > +
> > +	ret_val = igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
> > +
> > +	return ret_val;
> > +}
> 
> Please, this is just unnecessary syntactic masterbation, simplify this
> to:
> 
> s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
> {
> 	return igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
> }
> 
> And:
> 
> > +void igb_release_nvm_i210(struct e1000_hw *hw)
> > +{
> > +
> > +	igb_release_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
> > +}
> 
> Please get rid of that unnecessary empty line.
> 
> I can really tell when someone is extremely careless when removing
> their debugging code, and I can almost guarentee that's what has
> happened here.

Dave,

I get this cleaned up for Carolyn.  I will wait to re-submit till later
tonight to ensure that there is no other feedback or comments that need
to be addressed.

Cheers,
Jeff

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

^ permalink raw reply

* Re: [PATCH v4 1/2] vhost-net: fix handle_rx buffer size
From: Eric W. Biederman @ 2012-05-11 23:05 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Basil Gor, David S. Miller, netdev
In-Reply-To: <20120508192732.GA28536@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, May 04, 2012 at 12:55:23PM +0400, Basil Gor wrote:
>> Take vlan header length into account, when vlan id is stored as
>> vlan_tci. Otherwise tagged packets comming from macvtap will be
>> truncated.
>> 
>> Signed-off-by: Basil Gor <basil.gor@gmail.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> This doesn't fix packet socket backends but that can be
> handled separately later.

Yes that seems good enough.

Let me recommend just taking the packet out of the socket queue
instead of calling the recvmsg method.  Since vhost/net.c is
already munging with the queue directly that should make
it easy to avoid the peculiarities of pf_packet sockets
for reporting vlan information.

Eric

^ permalink raw reply

* Re: [PATCH net-next] sch_htb: report backlog information in htb classes
From: Eric Dumazet @ 2012-05-11 22:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1336775494.31653.297.camel@edumazet-glaptop>

On Sat, 2012-05-12 at 00:31 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> htb classes at level 0 can copy their qdisc child backlog to provide to
> "tc -s class" users more relevant information. (packet counter is
> correct but byte counter (backlog) is 0)
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/sched/sch_htb.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Please disregard this patch, more work is needed.

Thanks

^ permalink raw reply

* Re: [PATCH RESEND 2/5] net: fec: adopt pinctrl support
From: David Miller @ 2012-05-11 22:31 UTC (permalink / raw)
  To: shawn.guo; +Cc: arnd, netdev, s.hauer, olof, dong.aisheng, linux-arm-kernel
In-Reply-To: <1336352040-28447-3-git-send-email-shawn.guo@linaro.org>

From: Shawn Guo <shawn.guo@linaro.org>
Date: Mon,  7 May 2012 08:53:57 +0800

> Cc: netdev@vger.kernel.org
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* [PATCH net-next] sch_htb: report backlog information in htb classes
From: Eric Dumazet @ 2012-05-11 22:31 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

htb classes at level 0 can copy their qdisc child backlog to provide to
"tc -s class" users more relevant information. (packet counter is
correct but byte counter (backlog) is 0)

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_htb.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 9d75b77..0f4d534 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1073,9 +1073,10 @@ static int htb_dump_class(struct Qdisc *sch, unsigned long arg,
 	spin_lock_bh(root_lock);
 	tcm->tcm_parent = cl->parent ? cl->parent->common.classid : TC_H_ROOT;
 	tcm->tcm_handle = cl->common.classid;
-	if (!cl->level && cl->un.leaf.q)
+	if (!cl->level && cl->un.leaf.q) {
 		tcm->tcm_info = cl->un.leaf.q->handle;
-
+		sch->qstats.backlog = cl->un.leaf.q->qstats.backlog;
+	}
 	nest = nla_nest_start(skb, TCA_OPTIONS);
 	if (nest == NULL)
 		goto nla_put_failure;

^ permalink raw reply related

* Re: [net-next 2/2] igb: Add Support for new i210/i211 devices.
From: David Miller @ 2012-05-11 22:27 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: carolyn.wyborny, netdev, gospo, sassmann
In-Reply-To: <1336723282-3130-3-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 11 May 2012 01:01:22 -0700

> +s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
> +{
> +	s32 ret_val;
> +
> +	ret_val = igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
> +
> +	return ret_val;
> +}

Please, this is just unnecessary syntactic masterbation, simplify this
to:

s32 igb_acquire_nvm_i210(struct e1000_hw *hw)
{
	return igb_acquire_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
}

And:

> +void igb_release_nvm_i210(struct e1000_hw *hw)
> +{
> +
> +	igb_release_swfw_sync_i210(hw, E1000_SWFW_EEP_SM);
> +}

Please get rid of that unnecessary empty line.

I can really tell when someone is extremely careless when removing
their debugging code, and I can almost guarentee that's what has
happened here.

^ permalink raw reply

* Re: [PATCH] ks8851: Update link status during link change interrupt
From: David Miller @ 2012-05-11 22:23 UTC (permalink / raw)
  To: sboyd; +Cc: netdev, linux-kernel, ben-linux
In-Reply-To: <1336690290-21304-1-git-send-email-sboyd@codeaurora.org>

From: Stephen Boyd <sboyd@codeaurora.org>
Date: Thu, 10 May 2012 15:51:30 -0700

> If a link change interrupt comes in we just clear the interrupt
> and continue along without notifying the upper networking layers
> that the link has changed. Use the mii_check_link() function to
> update the link status whenever a link change interrupt occurs.
> 
> Cc: Ben Dooks <ben-linux@fluff.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

Applied, thanks.

^ permalink raw reply

* [PATCH net-next] etherdevice: Remove now unused compare_ether_addr_64bits
From: Joe Perches @ 2012-05-11 22:21 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <20120510.233554.263340913611469229.davem@davemloft.net>

Move and invert the logic from the otherwise unused
compare_ether_addr_64bits to ether_addr_equal_64bits.

Neaten the logic in is_etherdev_addr.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/linux/etherdevice.h |   46 ++++++++++++------------------------------
 1 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index afacf85..dc85c1d 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -193,12 +193,12 @@ static inline unsigned long zap_last_2bytes(unsigned long value)
 }
 
 /**
- * compare_ether_addr_64bits - Compare two Ethernet addresses
+ * ether_addr_equal_64bits - Compare two Ethernet addresses
  * @addr1: Pointer to an array of 8 bytes
  * @addr2: Pointer to an other array of 8 bytes
  *
- * Compare two ethernet addresses, returns 0 if equal, non-zero otherwise.
- * Unlike memcmp(), it doesn't return a value suitable for sorting.
+ * Compare two ethernet addresses, returns true if equal, false otherwise.
+ *
  * The function doesn't need any conditional branches and possibly uses
  * word memory accesses on CPU allowing cheap unaligned memory reads.
  * arrays = { byte1, byte2, byte3, byte4, byte6, byte7, pad1, pad2}
@@ -206,45 +206,25 @@ static inline unsigned long zap_last_2bytes(unsigned long value)
  * Please note that alignment of addr1 & addr2 is only guaranted to be 16 bits.
  */
 
-static inline unsigned compare_ether_addr_64bits(const u8 addr1[6+2],
-						 const u8 addr2[6+2])
+static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
+					   const u8 addr2[6+2])
 {
 #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
 	unsigned long fold = ((*(unsigned long *)addr1) ^
 			      (*(unsigned long *)addr2));
 
 	if (sizeof(fold) == 8)
-		return zap_last_2bytes(fold) != 0;
+		return zap_last_2bytes(fold) == 0;
 
 	fold |= zap_last_2bytes((*(unsigned long *)(addr1 + 4)) ^
 				(*(unsigned long *)(addr2 + 4)));
-	return fold != 0;
+	return fold == 0;
 #else
-	return compare_ether_addr(addr1, addr2);
+	return ether_addr_equal(addr1, addr2);
 #endif
 }
 
 /**
- * ether_addr_equal_64bits - Compare two Ethernet addresses
- * @addr1: Pointer to an array of 8 bytes
- * @addr2: Pointer to an other array of 8 bytes
- *
- * Compare two ethernet addresses, returns true if equal, false otherwise.
- *
- * The function doesn't need any conditional branches and possibly uses
- * word memory accesses on CPU allowing cheap unaligned memory reads.
- * arrays = { byte1, byte2, byte3, byte4, byte6, byte7, pad1, pad2}
- *
- * Please note that alignment of addr1 & addr2 is only guaranted to be 16 bits.
- */
-
-static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
-					   const u8 addr2[6+2])
-{
-	return !compare_ether_addr_64bits(addr1, addr2);
-}
-
-/**
  * is_etherdev_addr - Tell if given Ethernet address belongs to the device.
  * @dev: Pointer to a device structure
  * @addr: Pointer to a six-byte array containing the Ethernet address
@@ -252,23 +232,23 @@ static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
  * Compare passed address with all addresses of the device. Return true if the
  * address if one of the device addresses.
  *
- * Note that this function calls compare_ether_addr_64bits() so take care of
+ * Note that this function calls ether_addr_equal_64bits() so take care of
  * the right padding.
  */
 static inline bool is_etherdev_addr(const struct net_device *dev,
 				    const u8 addr[6 + 2])
 {
 	struct netdev_hw_addr *ha;
-	int res = 1;
+	bool res = false;
 
 	rcu_read_lock();
 	for_each_dev_addr(dev, ha) {
-		res = compare_ether_addr_64bits(addr, ha->addr);
-		if (!res)
+		res = ether_addr_equal_64bits(addr, ha->addr);
+		if (res)
 			break;
 	}
 	rcu_read_unlock();
-	return !res;
+	return res;
 }
 #endif	/* __KERNEL__ */
 

^ permalink raw reply related

* Re: [PATCH v2 net-next] fq_codel: Fair Queue Codel AQM
From: David Miller @ 2012-05-11 22:17 UTC (permalink / raw)
  To: eric.dumazet
  Cc: xiaosuo, netdev, dave.taht, nichols, van, therbert, mattmathis,
	ycheng, shemminger, maze, nanditad
In-Reply-To: <1336774576.31653.289.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 12 May 2012 00:16:16 +0200

> On Fri, 2012-05-11 at 21:30 +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
> 
> ...
> 
>> +static int fq_codel_dump_class_stats(struct Qdisc *sch, unsigned long cl,
>> +				     struct gnet_dump *d)
>> +{
>> +	struct fq_codel_sched_data *q = qdisc_priv(sch);
>> +	u32 idx = cl - 1;
>> +	struct gnet_stats_queue qs = { 0 };
>> +	struct tc_fq_codel_xstats xstats;
>> +
>> +	WARN_ON_ONCE(1);
>> +	if (idx < q->flows_cnt) {
>> +		const struct fq_codel_flow *flow = &q->flows[idx];
>> +		const struct sk_buff *skb = flow->head;
> 
> Oh well, I forgot to remove this WARN_ON_ONCE(1)

I can do it.

^ permalink raw reply

* Re: [PATCH v4 2/2] macvtap: restore vlan header on user read
From: David Miller @ 2012-05-11 22:17 UTC (permalink / raw)
  To: basil.gor; +Cc: mst, ebiederm, netdev
In-Reply-To: <1336121724-31902-2-git-send-email-basil.gor@gmail.com>

From: Basil Gor <basil.gor@gmail.com>
Date: Fri,  4 May 2012 12:55:24 +0400

> Ethernet vlan header is not on the packet and kept in the skb->vlan_tci
> when it comes from lower dev. This patch inserts vlan header in user
> buffer during skb copy on user read.
> 
> Signed-off-by: Basil Gor <basil.gor@gmail.com>

Also applied, thanks.

^ permalink raw reply

* Re: [PATCH v4 1/2] vhost-net: fix handle_rx buffer size
From: David Miller @ 2012-05-11 22:17 UTC (permalink / raw)
  To: basil.gor; +Cc: mst, ebiederm, netdev
In-Reply-To: <1336121724-31902-1-git-send-email-basil.gor@gmail.com>

From: Basil Gor <basil.gor@gmail.com>
Date: Fri,  4 May 2012 12:55:23 +0400

> Take vlan header length into account, when vlan id is stored as
> vlan_tci. Otherwise tagged packets comming from macvtap will be
> truncated.
> 
> Signed-off-by: Basil Gor <basil.gor@gmail.com>

My patience with Eric B. to review this has run out, applied,
thanks Basil.

^ permalink raw reply

* Re: [PATCH v2 net-next] fq_codel: Fair Queue Codel AQM
From: Eric Dumazet @ 2012-05-11 22:16 UTC (permalink / raw)
  To: Changli Gao
  Cc: David Miller, netdev, Dave Taht, Kathleen Nichols, Van Jacobson,
	Tom Herbert, Matt Mathis, Yuchung Cheng, Stephen Hemminger,
	Maciej Żenczykowski, Nandita Dukkipati
In-Reply-To: <1336764650.31653.277.camel@edumazet-glaptop>

On Fri, 2012-05-11 at 21:30 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>

...

> +static int fq_codel_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> +				     struct gnet_dump *d)
> +{
> +	struct fq_codel_sched_data *q = qdisc_priv(sch);
> +	u32 idx = cl - 1;
> +	struct gnet_stats_queue qs = { 0 };
> +	struct tc_fq_codel_xstats xstats;
> +
> +	WARN_ON_ONCE(1);
> +	if (idx < q->flows_cnt) {
> +		const struct fq_codel_flow *flow = &q->flows[idx];
> +		const struct sk_buff *skb = flow->head;

Oh well, I forgot to remove this WARN_ON_ONCE(1)

^ permalink raw reply

* Re: [net-next 2/2] stmmac: add mixed burst for DMA
From: David Miller @ 2012-05-11 22:12 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1336381953-18041-2-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Mon,  7 May 2012 11:12:33 +0200

> -	int (*init) (void __iomem *ioaddr, int pbl, int fb, int burst_len,
> -			u32 dma_tx, u32 dma_rx);
> +	int (*init) (void __iomem *ioaddr, int pbl, int fb, int mb,
> +			int burst_len, u32 dma_tx, u32 dma_rx);

Fix the indentation of the arguments on the second line,
the first character must line up with the first column after
the function's openning parenthesis on the previous line.

> -static int dwmac1000_dma_init(void __iomem *ioaddr, int pbl, int fb,
> +static int dwmac1000_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
>  			      int burst_len, u32 dma_tx, u32 dma_rx)

While you're here fix up that issue in the existing code here as well.

> -static int dwmac100_dma_init(void __iomem *ioaddr, int pbl, int fb,
> +static int dwmac100_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
>  			     int burst_len, u32 dma_tx, u32 dma_rx)

Likewise.

> -	int pbl = DEFAULT_DMA_PBL, fixed_burst = 0, burst_len = 0;
> +	int pbl = DEFAULT_DMA_PBL, fixed_burst = 0, burst_len = 0,
> +	    mixed_burst = 0;

This is gross, just make a new "int" declaration for mixed_burst.

^ permalink raw reply

* Linux Plumbers Conference: Networking-bufferbloat
From: Tom Herbert @ 2012-05-11 22:12 UTC (permalink / raw)
  To: Linux Netdev List

Hi all,

I proposed a micro conference for this years Linux Plumbers Conference
for August 29-31, 2012 San Diego, California.

I'm soliciting  ideas for presentation and conversation to fill a ~2.5
hour mini-conference.  I think the major topics will be buffer bloat
(can't wait to hear more about codel :-) ) , networking performance
(including TCP enhancements), and maybe integration advanced NIC HW
features.

If you have ideas for discussion that you think would benefit from
discussion at the Plumbers Conference, please add your idea to the
wiki, and email me.

http://www.linuxplumbersconf.org/2012/
http://wiki.linuxplumbersconf.org/2012:networking-bufferbloat

Thanks,
Tom

^ permalink raw reply

* Re: [net-next 1/2] stmmac: extend mac addr reg and fix perfect filering
From: David Miller @ 2012-05-11 22:11 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: netdev, gianni.antoniazzi-ext
In-Reply-To: <1336381953-18041-1-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Mon,  7 May 2012 11:12:32 +0200

> +	/* Extra 16 regs are available in cores newer than the 3.40. */
> +	if (id > 34)

No magic constants, even if you describe it in the comment.  Add
an appropriate macro and use it.
> +	/* For MAC Addr registers se have to set the Address Enable (AE)
> +	 * bit that has no effect on the High Reg 0 where the bit 31 (MO)
> +	 * is RO. */

Comments should be formatted as:

	/* For MAC Addr registers se have to set the Address Enable (AE)
	 * bit that has no effect on the High Reg 0 where the bit 31 (MO)
	 * is RO.
	 */

^ permalink raw reply

* Re: pch_gbe: oops with vlan (new)
From: Eric Dumazet @ 2012-05-11 22:10 UTC (permalink / raw)
  To: Andy Cress; +Cc: netdev
In-Reply-To: <1336770777.31653.283.camel@edumazet-glaptop>

On Fri, 2012-05-11 at 23:13 +0200, Eric Dumazet wrote:
> On Fri, 2012-05-11 at 13:48 -0700, Andy Cress wrote:
> > Folks,
> > 
> > I am looking for help in debugging a pch_gbe driver oops/abort.
> > 
> > Kernel: version 2.6.32-220.el6.i686 (RHEL6.2)
> > Driver: pch_gbe version 0.91-NAPI  (source tarball we used is at
> > https://sendfile.kontron.com/message/24tdUi6MXklnUtBLnOsumq until May
> > 16)
> > NIC: 0b:00.1 Ethernet controller [0200]: Intel Corporation Platform
> > Controller Hub EG20T Gigabit Ethernet Controller [8086:8802] (rev 02)
> > 
> > Configuration, with VLAN:
> >  eth0 (not started)
> >  eth0.100 = 192.168.100.1
> >  eth0.200 = 192.168.200.1
> >  eth0.6  = 192.168.6.1
> > 
> > When starting the VLAN configuration, then doing a ping test for >= 5
> > minutes, I get a kernel oop/abort message as shown below.  This does not
> > happen without configuring VLAN.
> > Where should I look for possible causes for a transmit queue timeout
> > like this?  
> > 
> > I have contacted the OKI/LAPIS driver authors, but no response so far.
> > I thought that this group might be able to comment from similar
> > experiences.
> > 
> > Andy
> 
> typical sign of a buggy driver
> 
> A quick look in current Linus tree show a non existent synchronization
> between ndo_start_xmit and TX completion.
> 
> tx completion uses a tx_queue_lock spinlock for nothing but false sense
> of correctness.

Please try the following patch : (based on current net-next tree)

Also this driver has a strange RX path : It does a copy of incoming
frames on fixed size skbs, (2048+overhead -> kmalloc-4096 pool) instead
of using skb of the right size...



 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h      |    2 
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |   25 ++++------
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
index 9f3dbc4..b07311e 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h
@@ -584,7 +584,6 @@ struct pch_gbe_hw_stats {
 /**
  * struct pch_gbe_adapter - board specific private data structure
  * @stats_lock:	Spinlock structure for status
- * @tx_queue_lock:	Spinlock structure for transmit
  * @ethtool_lock:	Spinlock structure for ethtool
  * @irq_sem:		Semaphore for interrupt
  * @netdev:		Pointer of network device structure
@@ -609,7 +608,6 @@ struct pch_gbe_hw_stats {
 
 struct pch_gbe_adapter {
 	spinlock_t stats_lock;
-	spinlock_t tx_queue_lock;
 	spinlock_t ethtool_lock;
 	atomic_t irq_sem;
 	struct net_device *netdev;
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 9dc7e50..3787c64 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -645,14 +645,11 @@ static void pch_gbe_mac_set_pause_packet(struct pch_gbe_hw *hw)
  */
 static int pch_gbe_alloc_queues(struct pch_gbe_adapter *adapter)
 {
-	int size;
-
-	size = (int)sizeof(struct pch_gbe_tx_ring);
-	adapter->tx_ring = kzalloc(size, GFP_KERNEL);
+	adapter->tx_ring = kzalloc(sizeof(*adapter->tx_ring), GFP_KERNEL);
 	if (!adapter->tx_ring)
 		return -ENOMEM;
-	size = (int)sizeof(struct pch_gbe_rx_ring);
-	adapter->rx_ring = kzalloc(size, GFP_KERNEL);
+
+	adapter->rx_ring = kzalloc(sizeof(*adapter->rx_ring), GFP_KERNEL);
 	if (!adapter->rx_ring) {
 		kfree(adapter->tx_ring);
 		return -ENOMEM;
@@ -1169,7 +1166,6 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 	struct sk_buff *tmp_skb;
 	unsigned int frame_ctrl;
 	unsigned int ring_num;
-	unsigned long flags;
 
 	/*-- Set frame control --*/
 	frame_ctrl = 0;
@@ -1216,14 +1212,14 @@ static void pch_gbe_tx_queue(struct pch_gbe_adapter *adapter,
 			}
 		}
 	}
-	spin_lock_irqsave(&tx_ring->tx_lock, flags);
+
 	ring_num = tx_ring->next_to_use;
 	if (unlikely((ring_num + 1) == tx_ring->count))
 		tx_ring->next_to_use = 0;
 	else
 		tx_ring->next_to_use = ring_num + 1;
 
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
+
 	buffer_info = &tx_ring->buffer_info[ring_num];
 	tmp_skb = buffer_info->skb;
 
@@ -1525,7 +1521,7 @@ pch_gbe_alloc_rx_buffers_pool(struct pch_gbe_adapter *adapter,
 						&rx_ring->rx_buff_pool_logic,
 						GFP_KERNEL);
 	if (!rx_ring->rx_buff_pool) {
-		pr_err("Unable to allocate memory for the receive poll buffer\n");
+		pr_err("Unable to allocate memory for the receive pool buffer\n");
 		return -ENOMEM;
 	}
 	memset(rx_ring->rx_buff_pool, 0, size);
@@ -1644,15 +1640,17 @@ pch_gbe_clean_tx(struct pch_gbe_adapter *adapter,
 	pr_debug("called pch_gbe_unmap_and_free_tx_resource() %d count\n",
 		 cleaned_count);
 	/* Recover from running out of Tx resources in xmit_frame */
+	spin_lock(&tx_ring->tx_lock);
 	if (unlikely(cleaned && (netif_queue_stopped(adapter->netdev)))) {
 		netif_wake_queue(adapter->netdev);
 		adapter->stats.tx_restart_count++;
 		pr_debug("Tx wake queue\n");
 	}
-	spin_lock(&adapter->tx_queue_lock);
+
 	tx_ring->next_to_clean = i;
-	spin_unlock(&adapter->tx_queue_lock);
+
 	pr_debug("next_to_clean : %d\n", tx_ring->next_to_clean);
+	spin_unlock(&tx_ring->tx_lock);
 	return cleaned;
 }
 
@@ -2043,7 +2041,6 @@ static int pch_gbe_sw_init(struct pch_gbe_adapter *adapter)
 		return -ENOMEM;
 	}
 	spin_lock_init(&adapter->hw.miim_lock);
-	spin_lock_init(&adapter->tx_queue_lock);
 	spin_lock_init(&adapter->stats_lock);
 	spin_lock_init(&adapter->ethtool_lock);
 	atomic_set(&adapter->irq_sem, 0);
@@ -2148,10 +2145,10 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 			 tx_ring->next_to_use, tx_ring->next_to_clean);
 		return NETDEV_TX_BUSY;
 	}
-	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 
 	/* CRC,ITAG no support */
 	pch_gbe_tx_queue(adapter, tx_ring, skb);
+	spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
 	return NETDEV_TX_OK;
 }
 

^ permalink raw reply related


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