Netdev List
 help / color / mirror / Atom feed
* Re: [net-next 05/13] igb: Support to read and export SFF-8472/8079 data
From: Ben Hutchings @ 2013-04-04 19:07 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Akeem G. Abodunrin, netdev, gospo, sassmann,
	Aurélien Guillaume
In-Reply-To: <1365075480-20183-6-git-send-email-jeffrey.t.kirsher@intel.com>

On Thu, 2013-04-04 at 04:37 -0700, Jeff Kirsher wrote:
> From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>
> 
> This patch adds support to read and export SFF-8472/8079 (SFP data)
> over i2c, through Ethtool.
[...]
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
[...]
> +static int igb_get_module_eeprom(struct net_device *netdev,
> +				 struct ethtool_eeprom *ee, u8 *data)
> +{
> +	struct igb_adapter *adapter = netdev_priv(netdev);
> +	struct e1000_hw *hw = &adapter->hw;
> +	u32 status = E1000_SUCCESS;
> +	u16 dataword = 0xFF;
> +	int i = 0;
> +
> +	/* Read the first block, SFF-8079, 2 bytes at a time */
> +	for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i += 2) {
> +		status = igb_read_phy_reg_i2c(hw, i, &dataword);
> +		if (status != E1000_SUCCESS)
> +			/* Error occurred while reading module */
> +			return -EIO;
> +
> +		data[i] = (dataword >> 8) & 0xFF;
> +		data[i+1] = dataword & 0xFF;
> +	}

What if ee->offset != 0 or ee->len < ETH_MODULE_SFF_8079_LEN?

> +	/* If the second block is requested, check if SFF-8472 is supported. */
> +	if (ee->len == ETH_MODULE_SFF_8472_LEN) {
> +		if (data[IGB_SFF_8472_COMP] == IGB_SFF_8472_UNSUP)
> +			return -EOPNOTSUPP;
> +
> +		/* Read the second block, SFF-8472, 2 bytes at a time */
> +		for (i = ETH_MODULE_SFF_8079_LEN;
> +		     i < ETH_MODULE_SFF_8472_LEN; i += 2) {
> +			status = igb_read_phy_reg_i2c(hw, i, &dataword);
> +			if (status != E1000_SUCCESS)
> +				return -EIO;
> +
> +			data[i] = (dataword >> 8) & 0xFF;
> +			data[i+1] = dataword & 0xFF;
> +		}
> +	}
[...]

What if ee->offset != 0 or ETH_MODULE_SFF_8079_LEN < ee->len <
ETH_MODULE_SFF_8472_LEN?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Fwd: New on this
From: Marc Marí @ 2013-04-04 19:57 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CA+nFU8RrfrYDwvRUCyjvu3BmxZFA98dozfHSn-hTPoMF4dOUBA@mail.gmail.com>

Some friends and I are trying to develop an application using AX.25
encoding, but not in standard situation.

It is intended to not use a TNC, and do everything by software, except
the transceiver itself, and by now, the transceiver can send data (not
AX.25, but plain data)

So now, we want to implement AX.25, so we can communicate with a
reciver which uses a TNC31. We've been looking a lot of documentation,
but we have not been able to determine what part of all the package
work will have to be done by hand.

We didn't configure AX25 yet, but we have the thought that what AX25
will return will be the data with the headers and CRC encapsulated in
KISS. Is that true?

So, we will have to do by hand the data whitening and scrambling
needed for the TNC?

Do you know any way to simplify all the process for a all-software transmission?

We are all a bit lost, and a bit of help would be very appreciated

Thank you in advance!

^ permalink raw reply

* [PATCH 0/2] changed irq handling and some cleanup for at86rf230
From: Sascha Herrmann @ 2013-04-04 21:01 UTC (permalink / raw)
  To: linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	netdev-u79uwXL29TY76Z2rM5mHXA

The first patch contains some cleanup for the at86rf230 driver. The
second patch changes the irq handling of the driver to avoid a
lockup caused by the irq handling of the driver.

Sascha Herrmann (2):
  at86rf230: remove unnecessary / dead code
  at86rf230: change irq handling to prevent lockups with edge type irq

 drivers/net/ieee802154/at86rf230.c |   31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

-- 
1.7.10.4


------------------------------------------------------------------------------
Minimize network downtime and maximize team effectiveness.
Reduce network management and security costs.Learn how to hire 
the most talented Cisco Certified professionals. Visit the 
Employer Resources Portal
http://www.cisco.com/web/learning/employer_resources/index.html

^ permalink raw reply

* [PATCH 2/2] at86rf230:  change irq handling to prevent lockups with edge type irq
From: Sascha Herrmann @ 2013-04-04 21:02 UTC (permalink / raw)
  To: linux-zigbee-devel, netdev
  Cc: alex.bluesman.smirnov, dbaryshkov, Sascha Herrmann
In-Reply-To: <cover.1365107512.git.sascha@ps.nvbi.de>

Hard code the interrupt type of the at86rf230 to rising edge type and
remove the calls to disable_irq_nosync() and enable_irq() from the
isr and the irqwork function. The at86rf230 resets the irq line only
after the irq status register is read. Disabling the irq can lock the
driver in situations where a irq is set by the radio while the driver
is still reading the frame buffer. Additional the irq filter register
is set to filter out all unused interrupts and the irq status register
is read in the probe function to clear the irq line.

Signed-off-by: Sascha Herrmann <sascha@ps.nvbi.de>
---
 drivers/net/ieee802154/at86rf230.c |   29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index fc315dd..eb5359f 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -51,7 +51,7 @@ struct at86rf230_local {
 	struct ieee802154_dev *dev;
 
 	spinlock_t lock;
-	bool irq_disabled;
+	bool irq_busy;
 	bool is_tx;
 };
 
@@ -544,7 +544,7 @@ at86rf230_xmit(struct ieee802154_dev *dev, struct sk_buff *skb)
 	unsigned long flags;
 
 	spin_lock(&lp->lock);
-	if  (lp->irq_disabled) {
+	if  (lp->irq_busy) {
 		spin_unlock(&lp->lock);
 		return -EBUSY;
 	}
@@ -705,20 +705,16 @@ static void at86rf230_irqwork(struct work_struct *work)
 	}
 
 	spin_lock_irqsave(&lp->lock, flags);
-	lp->irq_disabled = 0;
+	lp->irq_busy = 0;
 	spin_unlock_irqrestore(&lp->lock, flags);
-
-	enable_irq(lp->spi->irq);
 }
 
 static irqreturn_t at86rf230_isr(int irq, void *data)
 {
 	struct at86rf230_local *lp = data;
 
-	disable_irq_nosync(irq);
-
 	spin_lock(&lp->lock);
-	lp->irq_disabled = 1;
+	lp->irq_busy = 1;
 	spin_unlock(&lp->lock);
 
 	schedule_work(&lp->irqwork);
@@ -748,12 +744,7 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
 		dev_info(&lp->spi->dev, "Status: %02x\n", status);
 	}
 
-	rc = at86rf230_write_subreg(lp, SR_IRQ_MASK, 0xff); /* IRQ_TRX_UR |
-							     * IRQ_CCA_ED |
-							     * IRQ_TRX_END |
-							     * IRQ_PLL_UNL |
-							     * IRQ_PLL_LOCK
-							     */
+	rc = at86rf230_write_subreg(lp, SR_IRQ_MASK, IRQ_TRX_END);
 	if (rc)
 		return rc;
 
@@ -819,7 +810,7 @@ static int at86rf230_probe(struct spi_device *spi)
 {
 	struct ieee802154_dev *dev;
 	struct at86rf230_local *lp;
-	u8 man_id_0, man_id_1;
+	u8 man_id_0, man_id_1, status;
 	int rc;
 	const char *chip;
 	int supported = 0;
@@ -928,11 +919,17 @@ static int at86rf230_probe(struct spi_device *spi)
 	if (rc)
 		goto err_gpio_dir;
 
-	rc = request_irq(spi->irq, at86rf230_isr, IRQF_SHARED,
+	rc = request_irq(spi->irq, at86rf230_isr,
+			 IRQF_SHARED | IRQF_TRIGGER_RISING,
 			 dev_name(&spi->dev), lp);
 	if (rc)
 		goto err_gpio_dir;
 
+	/* Read irq status register to reset irq line */
+	rc = at86rf230_read_subreg(lp, RG_IRQ_STATUS, 0xff, 0, &status);
+	if (rc)
+		goto err_irq;
+
 	rc = ieee802154_register_device(lp->dev);
 	if (rc)
 		goto err_irq;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 1/2] at86rf230: remove unnecessary / dead code
From: Sascha Herrmann @ 2013-04-04 21:02 UTC (permalink / raw)
  To: linux-zigbee-devel, netdev
  Cc: alex.bluesman.smirnov, dbaryshkov, Sascha Herrmann
In-Reply-To: <cover.1365107512.git.sascha@ps.nvbi.de>

In at86rf230_probe() lp was first set to dev->priv and a few lines later
dev->priv was set to lp again, without changing lp in between. The call
to ieee802154_unregister_device() before err_irq: was unreachable.

Signed-off-by: Sascha Herrmann <sascha@ps.nvbi.de>
---
 drivers/net/ieee802154/at86rf230.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 6e88eab..fc315dd 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -838,7 +838,6 @@ static int at86rf230_probe(struct spi_device *spi)
 
 	lp->spi = spi;
 
-	dev->priv = lp;
 	dev->parent = &spi->dev;
 	dev->extra_tx_headroom = 0;
 	/* We do support only 2.4 Ghz */
@@ -940,7 +939,6 @@ static int at86rf230_probe(struct spi_device *spi)
 
 	return rc;
 
-	ieee802154_unregister_device(lp->dev);
 err_irq:
 	free_irq(spi->irq, lp);
 	flush_work(&lp->irqwork);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net-next 3/3] net/mlx4_en: Enable open-lldp DCB support
From: Or Gerlitz @ 2013-04-04 21:18 UTC (permalink / raw)
  To: John Fastabend; +Cc: Sagi Grimberg, Or Gerlitz, davem, netdev, amirv
In-Reply-To: <515DCDF2.8070304@intel.com>

 John Fastabend <john.r.fastabend@intel.com> wrote:
> On 4/4/2013 9:08 AM, Sagi Grimberg wrote:
>> On 4/4/2013 6:58 PM, John Fastabend wrote:
>>> On 4/4/2013 7:26 AM, Or Gerlitz wrote:

>>> Does lldpad work now with the mlx4_en driver?
>>> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>

>> Yes.
>> -Sagi

> So I looked at your code and lldpad a bit closer. I think this
> just works around a bug in lldpad. Also with this patch lldpad
> would try to xmit lldp pkts which would conflict with your firmware agent right?

We don't use firmware agent


> For IEEE DCBX modes lldpad should check the return values from
> the getdcbx dcbnl_rtnl_ops which you already have filled out. I
> would expect your driver to return DCB_CAP_DCBX_LLD_MANAGED to
> indicate it is being managed by firmware.


> See bnx2x_dcbnl_get_dcbx() for an example of an implementation
> which I believe is correct. I'll fix the user space lldpad bug.

can you be more specific what's the user space bug?

> Looks like I added my reveiwed-by line too quickly.

With this patch I can get the following

# lldptool -i eth3 -T -V ETS-CFG
tsa=0:ets,1:ets,2:ets,3:ets,4:ets,5:ets,6:ets,7:ets
up2tc=0:0,1:1,2:2,3:3,4:4,5:5,6:6,7:7 tcbw=12,12,12,12,13,13,13,13
TSA = 0:ets 1:ets 2:ets 3:ets 4:ets 5:ets 6:ets 7:ets
up2tc = 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
tcbw = 12% 12% 12% 12% 13% 13% 13% 13%

# lldptool -i eth3 -t -V ETS-CFG
IEEE 8021QAZ ETS Configuration TLV
         Willing: yes
         CBS: not supported
         MAX_TCS: 8
         PRIO_MAP: 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
         TC Bandwidth: 12% 12% 12% 12% 13% 13% 13% 13%
         TSA_MAP: 0:ets 1:ets 2:ets 3:ets 4:ets 5:ets 6:ets 7:ets

where without it, an error is returned, so basically, the patch allows
1st and most to do gets/sets where without it, it doesn't work,
anything wrong with that?

Or.

^ permalink raw reply

* Re: [PATCH 1/5 v2] mv643xx_eth: add Device Tree bindings
From: Simon Baatz @ 2013-04-04 21:29 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: davem, thomas.petazzoni, moinejf, jason, andrew, netdev,
	devicetree-discuss, rob.herring, grant.likely, jogo, buytenh, jm,
	linux-arm-kernel, sebastian.hesselbarth
In-Reply-To: <1365071235-11611-2-git-send-email-florian@openwrt.org>

Hi Florian

On Thu, Apr 04, 2013 at 12:27:11PM +0200, Florian Fainelli wrote:
> This patch adds Device Tree bindings following the already defined
> bindings at Documentation/devicetree/bindings/marvell.txt. The binding
> documentation is also enhanced with new optionnal properties required
> for supporting certain devices (RX/TX queue and SRAM). Since we now have
> proper support for the orion MDIO bus driver, there is no need to fiddle
> around with device tree phandles. PHY-less (MAC connected to switch)
> configurations are supported by not specifying any phy phandle for an
> ethernet node.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> - properly ifdef of_platform_bus_probe with CONFIG_OF
> - handle of_platform_bus_probe errors and cleanup accordingly
> - use of_property_read_u32 where applicable
> - parse "duplex" and "speed" property in PHY-less configuration
> 
>  Documentation/devicetree/bindings/marvell.txt |   25 +++++-
>  drivers/net/ethernet/marvell/mv643xx_eth.c    |  120 ++++++++++++++++++++++++-
>  2 files changed, 140 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/marvell.txt b/Documentation/devicetree/bindings/marvell.txt
> index f1533d9..e70a013 100644
> --- a/Documentation/devicetree/bindings/marvell.txt
> +++ b/Documentation/devicetree/bindings/marvell.txt
> @@ -112,9 +112,14 @@ prefixed with the string "marvell,", for Marvell Technology Group Ltd.
>     Required properties:
>       - #address-cells : <1>
>       - #size-cells : <0>
> -     - compatible : "marvell,mv64360-eth-block"
> +     - compatible : "marvell,mv64360-eth-block", "marvell,mv64360-eth-group",
> +		    "marvell,mv643xx-eth-block"
>       - reg : Offset and length of the register set for this block
>  
> +   Optional properties:
> +     - tx-csum-limit : Hardware limit above which transmit checksumming
> +                       is disabled.
> +
>     Example Discovery Ethernet block node:
>       ethernet-block@2000 {
>  	     #address-cells = <1>;
> @@ -130,7 +135,7 @@ prefixed with the string "marvell,", for Marvell Technology Group Ltd.
>  
>     Required properties:
>       - device_type : Should be "network".
> -     - compatible : Should be "marvell,mv64360-eth".
> +     - compatible : Should be "marvell,mv64360-eth", "marvell,mv643xx-eth".
>       - reg : Should be <0>, <1>, or <2>, according to which registers
>         within the silicon block the device uses.
>       - interrupts : <a> where a is the interrupt number for the port.
> @@ -140,6 +145,22 @@ prefixed with the string "marvell,", for Marvell Technology Group Ltd.
>         controller.
>       - local-mac-address : 6 bytes, MAC address
>  
> +   Optional properties:
> +     - clocks : Phandle to the clock control device and gate bit
> +     - clock-names : String describing the clock gate bit
> +     - speed : Speed to force the link (10, 100, 1000), used when no
> +               phy property is defined
> +     - duplex : Duplex to force the link (0: half, 1: full), used when no
> +               phy property is defined
> +     - rx-queue-count : number of RX queues to use
> +     - tx-queue-count : number of TX queues to use
> +     - rx-queue-size : size of the RX queue (in bytes)
> +     - tx-queue-size : size of the TX queue (in bytes)
> +     - rx-sram-addr : address of the SRAM for RX path (non 0 means used)
> +     - rx-sram-size : size of the SRAM for RX path (non 0 means used)
> +     - tx-sram-addr : address of the SRAM for TX path (non 0 means used)
> +     - tx-sram-size : size of the SRAM for TX path (non 0 means used)
> +
>     Example Discovery Ethernet port node:
>       ethernet@0 {
>  	     device_type = "network";
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index aedbd82..75599a8 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -60,6 +60,10 @@
>  #include <linux/inet_lro.h>
>  #include <linux/slab.h>
>  #include <linux/clk.h>
> +#include <linux/stringify.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_net.h>
>  
>  static char mv643xx_eth_driver_name[] = "mv643xx_eth";
>  static char mv643xx_eth_driver_version[] = "1.4";
> @@ -2542,14 +2546,23 @@ static void infer_hw_params(struct mv643xx_eth_shared_private *msp)
>  	}
>  }
>  
> +static const struct of_device_id mv643xx_eth_match[] = {
> +	{ .compatible = "marvell,mv64360-eth" },
> +	{ .compatible = "marvell,mv643xx-eth" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mv643xx_eth_match);
> +
>  static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>  {
>  	static int mv643xx_eth_version_printed;
> +	struct device_node *np = pdev->dev.of_node;
>  	struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
>  	struct mv643xx_eth_shared_private *msp;
>  	const struct mbus_dram_target_info *dram;
>  	struct resource *res;
>  	int ret;
> +	int tx_csum_limit = 0;
>  
>  	if (!mv643xx_eth_version_printed++)
>  		pr_notice("MV-643xx 10/100/1000 ethernet driver version %s\n",

This is not related to your change, but there is a problem in this
function that has already been discussed in the past if I remember
correctly: The respective clock needs to be enabled here (at least
on Kirkwood), since accesses to the hardware are done below. 
Enabling the clock only in mv643xx_eth_probe() is too late.

As said, this is not a problem introduced by your changes (and which
is currently circumvented by enabling the respective clocks in
kirkwood_legacy_clk_init() and kirkwood_ge0x_init()), but we might
want to fix this now to get rid of unconditionally enabling the GE
clocks in the DT case.


> @@ -2576,13 +2589,23 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>  	if (dram)
>  		mv643xx_eth_conf_mbus_windows(msp, dram);
>  
> -	msp->tx_csum_limit = (pd != NULL && pd->tx_csum_limit) ?
> -					pd->tx_csum_limit : 9 * 1024;
> +	if (np)
> +		of_property_read_u32(np, "tx-csum-limit", &tx_csum_limit);
> +	else
> +		tx_csum_limit = pd->tx_csum_limit;
> +
> +	msp->tx_csum_limit = tx_csum_limit ? tx_csum_limit : 9 * 1024;
>  	infer_hw_params(msp);
>  
>  	platform_set_drvdata(pdev, msp);
> +	ret = 0;
>  
> -	return 0;
> +#ifdef CONFIG_OF
> +	ret = of_platform_bus_probe(np, mv643xx_eth_match, &pdev->dev);
> +	if (ret)
> +		goto out_free;
> +#endif
> +	return ret;
>  
>  out_free:
>  	kfree(msp);
> @@ -2600,12 +2623,22 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id mv643xx_eth_shared_match[] = {
> +	{ .compatible = "marvell,mv64360-eth-group" },
> +	{ .compatible = "marvell,mv64360-eth-block" },
> +	{ .compatible = "marvell,mv643xx-eth-group" },
> +	{ .compatible = "marvell,mv643xx-eth-block" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mv643xx_eth_shared_match);
> +
>  static struct platform_driver mv643xx_eth_shared_driver = {
>  	.probe		= mv643xx_eth_shared_probe,
>  	.remove		= mv643xx_eth_shared_remove,
>  	.driver = {
>  		.name	= MV643XX_ETH_SHARED_NAME,
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(mv643xx_eth_shared_match),
>  	},
>  };
>  
> @@ -2764,6 +2797,74 @@ static const struct net_device_ops mv643xx_eth_netdev_ops = {
>  #endif
>  };
>  
> +#ifdef CONFIG_OF
> +static int mv643xx_eth_of_probe(struct platform_device *pdev)
> +{
> +	struct mv643xx_eth_platform_data *pd;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *shared = of_get_parent(np);
> +	struct device_node *phy_node;
> +	const int *prop;
> +	const char *mac_addr;
> +
> +	if (!pdev->dev.of_node)
> +		return 0;
> +
> +	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> +	if (!pd)
> +		return -ENOMEM;
> +
> +	pdev->dev.platform_data = pd;
> +
> +	pd->shared = of_find_device_by_node(shared);
> +	if (!pd->shared)
> +		return -ENODEV;
> +
> +	prop = of_get_property(np, "reg", NULL);
> +	if (!prop)
> +		return -EINVAL;
> +
> +	pd->port_number = be32_to_cpup(prop);
> +
> +	phy_node = of_parse_phandle(np, "phy", 0);
> +	if (!phy_node) {
> +		pd->phy_addr = MV643XX_ETH_PHY_NONE;
> +
> +		of_property_read_u32(np, "speed", &pd->speed);
> +		of_property_read_u32(np, "duplex", &pd->duplex);
> +	} else {
> +		prop = of_get_property(phy_node, "reg", NULL);
> +		if (prop)
> +			pd->phy_addr = be32_to_cpup(prop);
> +	}
> +
> +	mac_addr = of_get_mac_address(np);
> +	if (mac_addr)
> +		memcpy(pd->mac_addr, mac_addr, ETH_ALEN);
> +
> +#define rx_tx_queue_sram_property(_name)			\
> +	prop = of_get_property(np, __stringify(_name), NULL);	\
> +	if (prop)						\
> +		pd->_name = be32_to_cpup(prop);
> +
> +	rx_tx_queue_sram_property(rx_queue_count);
> +	rx_tx_queue_sram_property(tx_queue_count);
> +	rx_tx_queue_sram_property(rx_queue_size);
> +	rx_tx_queue_sram_property(tx_queue_size);
> +	rx_tx_queue_sram_property(rx_sram_addr);
> +	rx_tx_queue_sram_property(rx_sram_size);
> +	rx_tx_queue_sram_property(tx_sram_addr);
> +	rx_tx_queue_sram_property(rx_sram_size);
> +
> +	return 0;
> +}
> +#else
> +static inline int mv643xx_eth_of_probe(struct platform_device *dev)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int mv643xx_eth_probe(struct platform_device *pdev)
>  {
>  	struct mv643xx_eth_platform_data *pd;
> @@ -2772,7 +2873,12 @@ static int mv643xx_eth_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int err;
>  
> +	err = mv643xx_eth_of_probe(pdev);
> +	if (err)
> +		return err;
> +
>  	pd = pdev->dev.platform_data;
> +
>  	if (pd == NULL) {
>  		dev_err(&pdev->dev, "no mv643xx_eth_platform_data\n");
>  		return -ENODEV;

You don't change the clk initialization here:

#if defined(CONFIG_HAVE_CLK)
	mp->clk = clk_get(&pdev->dev, (pdev->id ? "1" : "0"));
	if (!IS_ERR(mp->clk)) {
		clk_prepare_enable(mp->clk);
		mp->t_clk = clk_get_rate(mp->clk);
	}
#endif

Which, if I understand correctly, works in the DT case because you
assign "clock-names" to the clocks in the DTS. However, I wonder
whether this works for any but the first Ethernet device.

In the old platform device setup, the pdev->id was set when
initialiazing the platform_device structure in common.c.  Where is
this done in the DT case?


In phy_scan(), the phy is searched like this:

		snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
				"orion-mdio-mii", addr);

		phydev = phy_connect(mp->dev, phy_id, mv643xx_eth_adjust_link,
				PHY_INTERFACE_MODE_GMII);

But "orion-mdio-mii:xx" is the name of the PHY if MDIO is setup via a
platform_device. I could not get this to work if the MDIO device is
setup via DT. Am I doing something wrong?


Additionally, in phy_scan() there is this:

	if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
		start = phy_addr_get(mp) & 0x1f;
		num = 32;
	} else {
	...

MV643XX_ETH_PHY_ADDR_DEFAULT is defined as 0. However, many Kirkwood
devices use "MV643XX_ETH_PHY_ADDR(0)".  If the module probe is
deferred in mv643xx_eth because the MDIO driver is not yet loaded,
all 32 PHY addresses are scanned without success.  This is not needed
and clutters the log.


- Simon

^ permalink raw reply

* [PATCH ethtool] realtek: Support the RTL-8168/8111B
From: Thierry Reding @ 2013-04-04 20:44 UTC (permalink / raw)
  To: netdev

This card can for instance be found in CompuLab TrimSlice devices.

Signed-off-by: Thierry Reding <thierry@gilfi.de>
---
 realtek.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/realtek.c b/realtek.c
index c3d7ae5..8ecd9a2 100644
--- a/realtek.c
+++ b/realtek.c
@@ -28,6 +28,7 @@ enum chip_type {
 	RTL8169_8110SB,
 	RTL8169_8110SCd,
 	RTL8169_8110SCe,
+	RTL8168_8111B,
 	RTL8168_8111Bb,
 	RTL8168_8111Bef,
 	RTL8101Ebc,
@@ -60,6 +61,7 @@ static struct chip_info {
 	{ "RTL-8169/8110SB",	HW_REVID(0, 0, 0, 1, 0, 0, 0, 0) },
 	{ "RTL-8169/8110SCd",	HW_REVID(0, 0, 0, 1, 1, 0, 0, 0) },
 	{ "RTL-8169/8110SCe",	HW_REVID(1, 0, 0, 1, 1, 0, 0, 0) },
+	{ "RTL-8168/8111B",	HW_REVID(0, 0, 1, 0, 1, 0, 0, 0) },
 	{ "RTL-8168/8111Bb",	HW_REVID(0, 0, 1, 1, 0, 0, 0, 0) },
 	{ "RTL-8168/8111Bef",	HW_REVID(0, 0, 1, 1, 1, 0, 0, 0) },
 	{ "RTL-8101Ebc",	HW_REVID(0, 0, 1, 1, 0, 1, 0, 0) },
-- 
1.8.2

^ permalink raw reply related

* Re: [PATCH 3/5 v2] ARM: kirkwood: add device node entries for the gigabit interfaces
From: Simon Baatz @ 2013-04-04 21:35 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: davem, thomas.petazzoni, moinejf, jason, andrew, netdev,
	devicetree-discuss, rob.herring, grant.likely, jogo, buytenh, jm,
	linux-arm-kernel, sebastian.hesselbarth
In-Reply-To: <1365071235-11611-4-git-send-email-florian@openwrt.org>

Hi Florian,

On Thu, Apr 04, 2013 at 12:27:13PM +0200, Florian Fainelli wrote:
> This patch modifies kirkwood.dtsi to specify the various gigabit
> interfaces nodes available on kirkwood devices. They are disabled by
> default and should be enabled on a per-board basis. egiga0 and egiga1
> aliases are defined for convenience. The mdio node is also present and
> should be enabled on a per-board basis as well.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> Changes since v1:
> - dropped change to arch/arm/mach-kirkwood/common.c to avoid merge conflicts

I think we should remove the clock aliases in
kirkwood_legacy_clk_init() in mach-kirkwood/dt-board.c (once we have
proper clock support, see my other mail).

> - fixed off-by 0x2000 ethernet-group nodes address
> 
>  arch/arm/boot/dts/kirkwood.dtsi |   46 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/kirkwood.dtsi b/arch/arm/boot/dts/kirkwood.dtsi
> index fada7e6..254f5a8 100644
> --- a/arch/arm/boot/dts/kirkwood.dtsi
> +++ b/arch/arm/boot/dts/kirkwood.dtsi
> @@ -7,6 +7,8 @@
>  	aliases {
>  	       gpio0 = &gpio0;
>  	       gpio1 = &gpio1;
> +	       egiga0 = &egiga0;
> +	       egiga1 = &egiga1;
>  	};
>  	intc: interrupt-controller {
>  		compatible = "marvell,orion-intc", "marvell,intc";
> @@ -202,5 +204,49 @@
>  			clocks = <&gate_clk 4>;
>  			status = "disabled";
>  		};
> +
> +		mdio@72004 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "marvell,orion-mdio";
> +			reg = <0x72004 0x84>;

Don't we need to add:

			interrupts = <46>;

here?


- Simon

^ permalink raw reply

* Re: [net-next PATCH V3] net: frag queue per hash bucket locking
From: David Miller @ 2013-04-04 21:38 UTC (permalink / raw)
  To: eric.dumazet; +Cc: brouer, netdev, fw, dborkman, hannes
In-Reply-To: <1365092655.3308.10.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Apr 2013 09:24:15 -0700

> On Thu, 2013-04-04 at 11:38 +0200, Jesper Dangaard Brouer wrote:
>> This patch implements per hash bucket locking for the frag queue
>> hash.  This removes two write locks, and the only remaining write
>> lock is for protecting hash rebuild.  This essentially reduce the
>> readers-writer lock to a rebuild lock.
> 
> Acked-by: Eric Dumazet <edumazet@google.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: pull request: wireless 2013-04-03
From: David Miller @ 2013-04-04 21:39 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20130403181647.GB31358@tuxdriver.com>

From: "John W. Linville" <linville@tuxdriver.com>
Date: Wed, 3 Apr 2013 14:16:48 -0400

> Here are some more fixes intended for the 3.9 stream...

Pulled, thanks John.

^ permalink raw reply

* Re: [PATCH 0/5] netfilter updates for net
From: David Miller @ 2013-04-04 21:42 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1365080169-6880-1-git-send-email-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu,  4 Apr 2013 14:56:04 +0200

> The following patchset contains netfilter updates for your net tree,
> they are:
> 
> * Fix missing the skb->trace reset in nf_reset, noticed by Gao Feng
>   while using the TRACE target with several net namespaces.
> 
> * Fix prefix translation in IPv6 NPT if non-multiple of 32 prefixes
>   are used, from Matthias Schiffer.
> 
> * Fix invalid nfacct objects with empty name, they are now rejected
>   with -EINVAL, spotted by Michael Zintakis, patch from myself.
> 
> * A couple of fixes for wrong return values in the error path of
>   nfnetlink_queue and nf_conntrack, from Wei Yongjun.
> 
> You can pull these changes from:
> 
> git://1984.lsi.us.es/nf master

Pulled, thanks Pablo.

^ permalink raw reply

* Re: [PATCH v2] r8169: fix auto speed down issue
From: David Miller @ 2013-04-04 21:46 UTC (permalink / raw)
  To: romieu; +Cc: hayeswang, netdev, linux-kernel, bowgotsai
In-Reply-To: <20130402213736.GA15328@electric-eye.fr.zoreil.com>

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue, 2 Apr 2013 23:37:36 +0200

> Hayes Wang <hayeswang@realtek.com> :
>> It would cause no link after suspending or shutdowning when the
>> nic changes the speed to 10M and connects to a link partner which
>> forces the speed to 100M.
>> 
>> Check the link partner ability to determine which speed to set.
>> 
>> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> 
> Acked-by: Francois Romieu <romieu@fr.zoreil.com>

Applied.

>> +static void rtl_speed_down(struct rtl8169_private *tp)
>> +{
>> +	u32	adv;
>> +	int	lpa;
>            ^^^^^
> Please use a single, true space (no tabs) next time.

I fixed this up when I applied this patch, thanks.

^ permalink raw reply

* Re: [PATCH v2 net-next 1/8] r8169: Remove firmware code
From: David Miller @ 2013-04-04 21:47 UTC (permalink / raw)
  To: hayeswang; +Cc: romieu, netdev, linux-kernel
In-Reply-To: <1364891022-3220-1-git-send-email-hayeswang@realtek.com>


Francois what is the status of this patch set?

I see there is still some discussion about dependencies between
patch #5 and #6, is that resolved?  If so, should I just apply
this series as-is?

Thanks!

^ permalink raw reply

* RE: [net-next 05/13] igb: Support to read and export SFF-8472/8079 data
From: Abodunrin, Akeem G @ 2013-04-04 22:17 UTC (permalink / raw)
  To: Ben Hutchings, Kirsher, Jeffrey T
  Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com, Aurélien Guillaume
In-Reply-To: <1365102471.2735.2.camel@bwh-desktop.uk.solarflarecom.com>



> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Thursday, April 04, 2013 12:08 PM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Abodunrin, Akeem G; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com; Aurélien Guillaume
> Subject: Re: [net-next 05/13] igb: Support to read and export SFF-8472/8079
> data
> 
> On Thu, 2013-04-04 at 04:37 -0700, Jeff Kirsher wrote:
> > From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>
> >
> > This patch adds support to read and export SFF-8472/8079 (SFP data)
> > over i2c, through Ethtool.
> [...]
> > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> [...]
> > +static int igb_get_module_eeprom(struct net_device *netdev,
> > +				 struct ethtool_eeprom *ee, u8 *data) {
> > +	struct igb_adapter *adapter = netdev_priv(netdev);
> > +	struct e1000_hw *hw = &adapter->hw;
> > +	u32 status = E1000_SUCCESS;
> > +	u16 dataword = 0xFF;
> > +	int i = 0;
> > +
> > +	/* Read the first block, SFF-8079, 2 bytes at a time */
> > +	for (i = 0; i < ETH_MODULE_SFF_8079_LEN; i += 2) {
> > +		status = igb_read_phy_reg_i2c(hw, i, &dataword);
> > +		if (status != E1000_SUCCESS)
> > +			/* Error occurred while reading module */
> > +			return -EIO;
> > +
> > +		data[i] = (dataword >> 8) & 0xFF;
> > +		data[i+1] = dataword & 0xFF;
> > +	}
> 
> What if ee->offset != 0 or ee->len < ETH_MODULE_SFF_8079_LEN?
> 
> > +	/* If the second block is requested, check if SFF-8472 is supported. */
> > +	if (ee->len == ETH_MODULE_SFF_8472_LEN) {
> > +		if (data[IGB_SFF_8472_COMP] == IGB_SFF_8472_UNSUP)
> > +			return -EOPNOTSUPP;
> > +
> > +		/* Read the second block, SFF-8472, 2 bytes at a time */
> > +		for (i = ETH_MODULE_SFF_8079_LEN;
> > +		     i < ETH_MODULE_SFF_8472_LEN; i += 2) {
> > +			status = igb_read_phy_reg_i2c(hw, i, &dataword);
> > +			if (status != E1000_SUCCESS)
> > +				return -EIO;
> > +
> > +			data[i] = (dataword >> 8) & 0xFF;
> > +			data[i+1] = dataword & 0xFF;
> > +		}
> > +	}
> [...]
> 
> What if ee->offset != 0 or ETH_MODULE_SFF_8079_LEN < ee->len <
> ETH_MODULE_SFF_8472_LEN?

Thanks Ben,
 
Yes, you are right... I believe this is an oversight. There may be need to use ee->offset as starting point and make sure ETH_MODULE_SFF_8079_LEN or ETH_MODULE_SFF_8472_LEN is within ee->len scope.

We would address this in the follow up patch.

Regards,

~Akeem
> 
> Ben.
> 
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's
> the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [Patch net-next] tcp: add a global sysctl to control TCP delayed ack
From: Hannes Frederic Sowa @ 2013-04-04 22:48 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, Eric Dumazet, Rick Jones, Stephen Hemminger,
	David S. Miller, Thomas Graf, David Laight
In-Reply-To: <1365070560-11544-1-git-send-email-amwang@redhat.com>

On Thu, Apr 04, 2013 at 06:16:00PM +0800, Cong Wang wrote:
> From: Cong Wang <amwang@redhat.com>
> 
> Change from RFC:
> * make the sysctl per netns
> 
> According to previous discussion, it seems there is no
> reasonable heuristics.
> 
> Similar to TCP_QUICK_ACK option, but for people who can't
> modify the source code and still wants to control
> TCP delayed ACK behavior.
> 
> David, do you still have any objection?

I totally understand the objections that were given regarding this
patch. But for defense of this patch we also provide a knob to disable
slow start after idle, which from my point of view is as "evil" as
this change.

Btw I think we should make tcp_slow_start_after_idle namespace aware, too.

^ permalink raw reply

* Re: Fwd: New on this
From: Ben Hutchings @ 2013-04-04 23:06 UTC (permalink / raw)
  To: Marc Marí; +Cc: netdev, linux-hams
In-Reply-To: <CA+nFU8SrVn3+3aGzsmAy3xexiCQEMHS9a3AKc4p7H79toz9W7Q@mail.gmail.com>

On Thu, 2013-04-04 at 21:57 +0200, Marc Marí wrote:
> Some friends and I are trying to develop an application using AX.25
> encoding, but not in standard situation.

The linux-hams@vger.kernel.org list is probably a better place to
discuss this.

Ben.

> It is intended to not use a TNC, and do everything by software, except
> the transceiver itself, and by now, the transceiver can send data (not
> AX.25, but plain data)
> 
> So now, we want to implement AX.25, so we can communicate with a
> reciver which uses a TNC31. We've been looking a lot of documentation,
> but we have not been able to determine what part of all the package
> work will have to be done by hand.
> 
> We didn't configure AX25 yet, but we have the thought that what AX25
> will return will be the data with the headers and CRC encapsulated in
> KISS. Is that true?
> 
> So, we will have to do by hand the data whitening and scrambling
> needed for the TNC?
> 
> Do you know any way to simplify all the process for a all-software transmission?
> 
> We are all a bit lost, and a bit of help would be very appreciated
> 
> Thank you in advance!

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [Patch net-next] tcp: add a global sysctl to control TCP delayed ack
From: David Miller @ 2013-04-04 23:25 UTC (permalink / raw)
  To: hannes
  Cc: amwang, netdev, eric.dumazet, rick.jones2, shemminger, tgraf,
	David.Laight
In-Reply-To: <20130404224810.GE23056@order.stressinduktion.org>

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 5 Apr 2013 00:48:10 +0200

> I totally understand the objections that were given regarding this
> patch. But for defense of this patch we also provide a knob to disable
> slow start after idle, which from my point of view is as "evil" as
> this change.

I completely disagree, slow start after idle is way too aggressively
throwing past history away, so turning that off is much safer.

^ permalink raw reply

* Re: [Patch net-next] tcp: add a global sysctl to control TCP delayed ack
From: Ben Greear @ 2013-04-04 23:39 UTC (permalink / raw)
  To: David Miller
  Cc: hannes, amwang, netdev, eric.dumazet, rick.jones2, shemminger,
	tgraf, David.Laight
In-Reply-To: <20130404.192539.1157364077083658871.davem@davemloft.net>

On 04/04/2013 04:25 PM, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Fri, 5 Apr 2013 00:48:10 +0200
>
>> I totally understand the objections that were given regarding this
>> patch. But for defense of this patch we also provide a knob to disable
>> slow start after idle, which from my point of view is as "evil" as
>> this change.
>
> I completely disagree, slow start after idle is way too aggressively
> throwing past history away, so turning that off is much safer.

I've been carrying a per-socket way to control delayed ack in my tree for a while,
and it is a big performance gain on wifi networks where the network is
basically half-duplex.  If I recall correctly, it's worth 50+Mbps
throughput improvement in some cases.  If it matters, I can post more
detailed numbers.

There are drawbacks when you set delayed ack too high:  The ramp-up time
for TCP takes a good bit longer.  But, some applications may want to trade
slower startup time for better bulk transport, and at moderate delayed-ack
values, the ramp up time is not noticeably impaired.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH v2 net-next 1/8] r8169: Remove firmware code
From: Francois Romieu @ 2013-04-04 23:42 UTC (permalink / raw)
  To: David Miller; +Cc: hayeswang, netdev, linux-kernel
In-Reply-To: <20130404.174717.2232220782916755499.davem@davemloft.net>

David Miller <davem@davemloft.net> :
[...]
> I see there is still some discussion about dependencies between
> patch #5 and #6, is that resolved?

Yes.

> If so, should I just apply this series as-is ?

Yes.

- the series is imho -stable unfriendly: whoever wants to figure what
  should be fed into a -stable branch will have a hard time. :o/
- the driver could had been more careful about firmware version/magic
  checks and firmware opcodes recycling. It's a bit late. It won't
  necessarily hurt.
- there is a whole release cycle ahead to find problems - if any - due
  to the hw_start flow change. It seems sane.
- the relative amount of binary like cruft is going down.

I am not overflowed with enthusiasm but the gain should exceed the pain.

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH] lib80211: make lib80211 can be enabled independently
From: Julian Calaby @ 2013-04-04 23:51 UTC (permalink / raw)
  To: Wang YanQing, Johannes Berg, linux-wireless, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <20130404160130.GA2577@udknight>

Hi Wang,

On Fri, Apr 5, 2013 at 3:01 AM, Wang YanQing <udknight@gmail.com> wrote:
>
> Current we can only enable lib80211 by enable a driver
> in tree use it which will select it, but some out tree's
> drivers also use it, so I think it has sense to make lib80211
> can be enabled independently.

Just as a bit of explanation for Johannes' NACK:

1. The only reason lib80211 still exists is because a couple of
in-tree drivers still use it. If this weren't the case, the code would
have been removed a long time ago as it's been completely replaced by
mac80211 and cfg80211. All modern drivers _must_ use mac80211 or
cfg80211 without exception.
2. In general, there is no official in-kernel support for
out-of-kernel drivers, regardless of quality, status or importance.
3. I believe that the in-tree brcmsmac driver already supports the
Broadcom card you reference.

Thanks,

--
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply

* Re: [PATCH net-next 3/3] net/mlx4_en: Enable open-lldp DCB support
From: John Fastabend @ 2013-04-04 23:54 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: John Fastabend, Sagi Grimberg, Or Gerlitz, davem, netdev, amirv
In-Reply-To: <CAJZOPZKms=25WUR5WL0DEZ7yAi=zOKjZEGdO1BQsyJ+tvPMe+g@mail.gmail.com>

On 04/04/2013 02:18 PM, Or Gerlitz wrote:
>   John Fastabend <john.r.fastabend@intel.com> wrote:
>> On 4/4/2013 9:08 AM, Sagi Grimberg wrote:
>>> On 4/4/2013 6:58 PM, John Fastabend wrote:
>>>> On 4/4/2013 7:26 AM, Or Gerlitz wrote:
>
>>>> Does lldpad work now with the mlx4_en driver?
>>>> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
>
>>> Yes.
>>> -Sagi
>
>> So I looked at your code and lldpad a bit closer. I think this
>> just works around a bug in lldpad. Also with this patch lldpad
>> would try to xmit lldp pkts which would conflict with your firmware agent right?
>
> We don't use firmware agent
>

Ah OK. Your mlx4_en_dcbnl_getdcbx() routine should also return
DCB_CAP_DCBX_HOST then just to be clear. I should write a patch
to clarify those defines I guess.

>
>> For IEEE DCBX modes lldpad should check the return values from
>> the getdcbx dcbnl_rtnl_ops which you already have filled out. I
>> would expect your driver to return DCB_CAP_DCBX_LLD_MANAGED to
>> indicate it is being managed by firmware.
>
>
>> See bnx2x_dcbnl_get_dcbx() for an example of an implementation
>> which I believe is correct. I'll fix the user space lldpad bug.
>
> can you be more specific what's the user space bug?
>

Sure.

In lldpad before the IEEE DCBx (802.1Qaz) TLVs are initialized and the
hardware is managed with the 802.1Qaz module there is a check to verify
the hardware actually supports 802.1Qaz because we don't have a SW
fall back.

Today in lldpad this check is done via a DCB_CMD_GCAP netlink msg. But
this message is really built for the older CEE version of DCBX. In
./include/net/dcbnl.h it is listed in the CEE ops section and it uses
the CEE defines such as *_PG_* instead of including the more precise
*_ETS_* attributes. It also does not give any indication of the
firmware status.

A better check for IEEE support would be to use the getdcbx op via
DCB_CMD_GDCBX which has an explicit define for IEEE/CEE and fw
or host managed.

	#define DCB_CAP_DCBX_HOST               0x01
	#define DCB_CAP_DCBX_LLD_MANAGED        0x02
	#define DCB_CAP_DCBX_VER_CEE            0x04
	#define DCB_CAP_DCBX_VER_IEEE           0x08
	#define DCB_CAP_DCBX_STATIC             0x10

If this check is used your driver would work as is and lldpad would be
able to learn in a single call if (a) the hardware supports DCB (b)
which version CEE or IEEE or both and (c) if the firmware is already
managing DCBX. This reduces driver code somewhat which is a nice thing
and is more precise.

So as a heads up I am planning to fix/improve lldpad to use these bits
and it looks like every driver implementing the IEEE dcbnl ops does in
fact implement the getdcbx op so it should not break any drivers and
be backwards compatible. Meaning running a new lldpad version on an old
kernel will still work.

>> Looks like I added my reveiwed-by line too quickly.
>
> With this patch I can get the following
>
> # lldptool -i eth3 -T -V ETS-CFG
> tsa=0:ets,1:ets,2:ets,3:ets,4:ets,5:ets,6:ets,7:ets
> up2tc=0:0,1:1,2:2,3:3,4:4,5:5,6:6,7:7 tcbw=12,12,12,12,13,13,13,13
> TSA = 0:ets 1:ets 2:ets 3:ets 4:ets 5:ets 6:ets 7:ets
> up2tc = 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
> tcbw = 12% 12% 12% 12% 13% 13% 13% 13%
>
> # lldptool -i eth3 -t -V ETS-CFG
> IEEE 8021QAZ ETS Configuration TLV
>           Willing: yes
>           CBS: not supported
>           MAX_TCS: 8
>           PRIO_MAP: 0:0 1:1 2:2 3:3 4:4 5:5 6:6 7:7
>           TC Bandwidth: 12% 12% 12% 12% 13% 13% 13% 13%
>           TSA_MAP: 0:ets 1:ets 2:ets 3:ets 4:ets 5:ets 6:ets 7:ets
>
> where without it, an error is returned, so basically, the patch allows
> 1st and most to do gets/sets where without it, it doesn't work,
> anything wrong with that?

Agree it doesn't work without this patch but we can fix it in user
space. This has the nice benefit of letting lldpad work with mlx4 on
older kernels. In general I think its easier to update user space then
the kernel for most users. I'll send you a lldpad patch in a few
moments.

Thanks,
John

>
> Or.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* Re: r8169 regression: UDP packets dropped intermittantly
From: Jonathan Woithe @ 2013-04-05  0:15 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, Jonathan Woithe
In-Reply-To: <20130311000216.GA31808@zoreil.com>

On Mon, Mar 11, 2013 at 01:02:16AM +0100, Francois Romieu wrote:
> Jonathan Woithe <jwoithe@atrad.com.au> :
> [...]
> > While we can rely on the retry system to paper over this regression, the
> > behaviour of earlier kernels suggests that there is a real issue at play. 
> > Is there any chance that this can be looked at ?
> 
> Yes.
> 
> You'll have to be a bit patient since I still have to spend a pair of non
> worked hours on a compromised system. :o/

Do you have any indication yet as to when this might be able to be looked
at?

> Take it for what it's worth but I don't test much with PREEMPT, NOHZ or
> SLUB and I never tried all those at the same time.

FYI I sent a followup to this where I determined that the regression is
still present without the above kernel configuration parameters enabled.

jonathan

^ permalink raw reply

* Re: [PATCH v2] MPLS: Add limited GSO support
From: Simon Horman @ 2013-04-05  1:07 UTC (permalink / raw)
  To: Jesse Gross; +Cc: dev@openvswitch.org, netdev, Jarno Rajahalme, Pravin B Shelar
In-Reply-To: <CAEP_g=-9sup3ny7Y8HKW85LrtM=xZfEme7UeKvGZ11uHUFqPcA@mail.gmail.com>

On Thu, Apr 04, 2013 at 10:20:47AM -0700, Jesse Gross wrote:
> On Wed, Apr 3, 2013 at 5:55 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Apr 03, 2013 at 05:44:17PM -0700, Jesse Gross wrote:
> >> On Wed, Apr 3, 2013 at 5:28 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Wed, Apr 03, 2013 at 04:51:38PM -0700, Jesse Gross wrote:
> >> >> On Wed, Apr 3, 2013 at 2:11 AM, Simon Horman <horms@verge.net.au> wrote:
> >> >> > In the case where a non-MPLS packet is recieved and an MPLS stack is
> >> >> > added it may well be the case that the original skb is GSO but the
> >> >> > NIC used for transmit does not support GSO of MPLS packets.
> >> >> >
> >> >> > The aim of this code is to provide GSO in software for MPLS packets
> >> >> > whose skbs are GSO.
> >> >> >
> >> >> > When an implementation adds an MPLS stack to a non-MPLS packet it should do
> >> >> > the following to skb metadata:
> >> >> >
> >> >> > * Set skb_mac_header(skb)->protocol to the new MPLS ethertype.
> >> >> >   That is, either ETH_P_MPLS_UC or ETH_P_MPLS_MC.
> >> >> >
> >> >> > * Leave skb->protocol as the old non-MPLS ethertype.
> >> >> >
> >> >> > * Set skb->encapsulation = 1.
> >> >> >
> >> >> >   This may not strictly be necessary as I believe that checking
> >> >> >   skb_mac_header(skb)->protocol and skb->protocol should be necessary and
> >> >> >   sufficient.
> >> >> >
> >> >> >   However, it does seem to fit nicely with the current implementation of
> >> >> >   dev_hard_start_xmit() where the more expensive check of
> >> >> >   skb_mac_header(skb)->protocol may be guarded by an existing check of
> >> >> >   skb->encapsulation.
> >> >> >
> >> >> > One aspect of this patch that I am unsure about is the modification I have
> >> >> > made to skb_segment(). This seems to be necessary as checskum accelearation
> >> >> > may no longer be possible as the packet has changed to be MPLS from some
> >> >> > other packet type which may have been supported by the hardware in use.
> >> >> >
> >> >> > I will post a patch, "[PATCH v3.24] datapath: Add basic MPLS support to
> >> >> > kernel" which adds MPLS support to the kernel datapath of Open vSwtich.
> >> >> > That patch sets the above requirements in
> >> >> > datapath/actions.c:set_ethertype() and was used to exercise the MPLS GSO
> >> >> > code. The datapath patch is against the Open vSwtich tree but it is
> >> >> > intended that it be added to the Open vSwtich code present in the mainline
> >> >> > Linux kernel at some point.
> >> >> >
> >> >> > Suggested by Jesse Gross. Based heavily on "v4 GRE: Add TCP segmentation
> >> >> > offload for GRE" by Pravin B Shelar.
> >> >> >
> >> >> > Cc: Jesse Gross <jesse@nicira.com>
> >> >> > Cc: Pravin B Shelar <pshelar@nicira.com>
> >> >> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >> >>
> >> >> MPLS is very similar to both the Ethernet header and vlans in that GSO
> >> >> only requires replication without any modification.  That means that
> >> >> if we look at the mac_len as containing all three then we can just
> >> >> copy it without any special knowledge.  I don't know that we carefully
> >> >> maintain mac_len in all places but you are already doing that in your
> >> >> MPLS patches.
> >> >
> >> > At least for the cases that I am aware of I think that mac_len is
> >> > predictable. But I'm a little unsure of what you are getting at here.
> >>
> >> If you have the MAC len then you don't need any new MPLS code here at
> >> all; just replicate the whole thing onto each packet.
> >
> > The MAC len is set to cover everything up to the top of the MPLS stack.
> > So it seems that something needs to be done to account for the length
> > of the MPLS stack.
> >
> > Are you suggesting that if Open vSwtich set up the MAC len to extend
> > to the end of the MPLS stack then mpls_gro_segment() would not be necessary?
> 
> Something along those lines.  I think this is very similar to the
> skb->protocol discussion (and likely influenced by the outcome of
> that).  MPLS is a little weird with respect to the existing layer
> pointers but if we can find a good definition then I think that the
> GSO code should be pretty minimal.

Thanks, I understand.

Mainly for the benefit of those who have not been exposed to MPLS (for Open
vSwtich) I will summarise how I see the problem with the layer pointers and
protocol values in relation to MPLS.


Basically MPLS sits between L2 and L3, and is sometimes referred to as
L2.5.  Currently the code makes use of the network_header in the skb to
point to the top of the MPLS label stack. But arguably it could just as
validly be used to point to the bottom of the MPLS label stack, where the
(non-MPLS) L3 data lies.

Up until now it has seemed to be more important to know where the top of
the MPLS label stack is, so using the network_header for that purpose has
worked out quite well in the Open vSwtich code that uses it ("[PATCH v3.24]
datapath: Add basic MPLS support to kernel").

We now see a situation where it would be useful to just know where the
bottom of the MPLS label stack lies.


The issue regarding skb->protocol and skb_mac_header(skb)->protocol is
that when an MPLS push occurs on a previously non-MPLS packet the
protocol is changed to either MPLS multicast or MPLS unicast. And more
importantly, the MPLS label stack entry doesn't include the old protocol
so it is no longer present anywhere in the packet. From an implementation
point of view this is a critical difference between MPLS and VLANs.

The way that Open vSwitch currently implements MPLS push results in:

* skb_mac_header(skb)->protocol set to the new, MPLS, protocol
* skb->protocol left as the old, (in the case relating to GSO non-MPLS),
  protocol

The MPLS GSO code I posted uses this information, plus the fact that
skb->encapsulation = 1 (not strictly necessary, I think), to determine
if MPLS GSO segmentation should be performed.


Jesse has suggested that there would be no need for MPLS GSO segmentation
code, or that at the very least it would be smaller, if the skb was set up
more cleverly, with skb->mac_len and skb->network_header corresponding to
the bottom of the MPLS label stack. This appears to tie into any decision
about the treatment of skb_mac_header(skb)->protocol and skb->protocol.

^ permalink raw reply

* [PATCH net-next] ip_gre: fix a possible crash in parse_gre_header()
From: Eric Dumazet @ 2013-04-05  1:41 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Pravin B Shelar

From: Eric Dumazet <edumazet@google.com>

pskb_may_pull() can change skb->head, so we must init iph/greh after
calling it.

Bug added in commit c54419321455 (GRE: Refactor GRE tunneling code.)

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Pravin B Shelar <pshelar@nicira.com>
---
 net/ipv4/ip_gre.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index e5dfd28..987a4e5 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -159,14 +159,14 @@ static int ip_gre_calc_hlen(__be16 o_flags)
 static int parse_gre_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 			    bool *csum_err, int *hdr_len)
 {
-	struct iphdr *iph = ip_hdr(skb);
-	struct gre_base_hdr *greh;
+	unsigned int ip_hlen = ip_hdrlen(skb);
+	const struct gre_base_hdr *greh;
 	__be32 *options;
 
 	if (unlikely(!pskb_may_pull(skb, sizeof(struct gre_base_hdr))))
 		return -EINVAL;
 
-	greh = (struct gre_base_hdr *)((u8 *)iph + (iph->ihl << 2));
+	greh = (struct gre_base_hdr *)(skb_network_header(skb) + ip_hlen);
 	if (unlikely(greh->flags & (GRE_VERSION | GRE_ROUTING)))
 		return -EINVAL;
 
@@ -176,6 +176,8 @@ static int parse_gre_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 	if (!pskb_may_pull(skb, *hdr_len))
 		return -EINVAL;
 
+	greh = (struct gre_base_hdr *)(skb_network_header(skb) + ip_hlen);
+
 	tpi->proto = greh->protocol;
 
 	options = (__be32 *)(greh + 1);

^ 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