Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 1/3] net/usb/r8152: support aggregation
From: Francois Romieu @ 2013-08-15 12:26 UTC (permalink / raw)
  To: Hayes Wang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb, David Miller
In-Reply-To: <1376484880-741-1-git-send-email-hayeswang@realtek.com>

Hayes Wang <hayeswang@realtek.com> :
[...]
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 11c51f2..abb0b9f 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[...]
> @@ -315,13 +318,34 @@ struct tx_desc {
>  	u32 opts2;
>  };
>  
> +struct rx_agg {
> +	struct list_head list;
> +	struct urb *urb;
> +	void *context;

void * is not needed: context is always struct r8152 *.

> +	void *buffer;
> +	void *head;
> +};
> +
> +struct tx_agg {
> +	struct list_head list;
> +	struct urb *urb;
> +	void *context;

void * is not needed: context is always struct r8152 *.

> +	void *buffer;
> +	void *head;
> +	u32 skb_num;
> +	u32 skb_len;
> +};
> +
>  struct r8152 {
>  	unsigned long flags;
>  	struct usb_device *udev;
>  	struct tasklet_struct tl;
>  	struct net_device *netdev;
> -	struct urb *rx_urb, *tx_urb;
> -	struct sk_buff *tx_skb, *rx_skb;
> +	struct tx_agg tx_info[RTL8152_MAX_TX];
> +	struct rx_agg rx_info[RTL8152_MAX_RX];
> +	struct list_head rx_done, tx_free;
> +	struct sk_buff_head tx_queue;
> +	spinlock_t rx_lock, tx_lock;

You may group rx data on one side and tx data on an other side to be
more cache friendly.

[...]
> @@ -686,6 +711,9 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16 data)
>  	ocp_write_word(tp, MCU_TYPE_PLA, ocp_index, data);
>  }
>  
> +static
> +int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags);
> +

It's a new, less than 10 lines function without driver internal dependencies.

The forward declaration is not needed.

[...]
> @@ -743,129 +751,425 @@ static struct net_device_stats *rtl8152_get_stats(struct net_device *dev)
>  
>  static void read_bulk_callback(struct urb *urb)

No rtl8152_ prefix ?

>  {
[...]
> -	if (!netif_device_present(netdev))
> +	if (!netif_carrier_ok(netdev))
>  		return;

How is it related to the subject of the patch ?

[...]
> +static void rx_bottom(struct r8152 *tp)
> +{
> +	struct net_device_stats *stats;
> +	struct net_device *netdev;
> +	struct rx_agg *agg;
> +	struct rx_desc *rx_desc;
> +	unsigned long lockflags;

Idiom: 'flags'.

> +	struct list_head *cursor, *next;
> +	struct sk_buff *skb;
> +	struct urb *urb;
> +	unsigned pkt_len;
> +	int len_used;
> +	u8 *rx_data;
> +	int ret;

The scope of these variables is needlessly wide.

> +
> +	netdev = tp->netdev;
> +
> +	stats = rtl8152_get_stats(netdev);
> +
> +	spin_lock_irqsave(&tp->rx_lock, lockflags);
> +	list_for_each_safe(cursor, next, &tp->rx_done) {
> +		list_del_init(cursor);
> +		spin_unlock_irqrestore(&tp->rx_lock, lockflags);
> +
> +		agg = list_entry(cursor, struct rx_agg, list);
> +		urb = agg->urb;
> +		if (urb->actual_length < ETH_ZLEN) {

			goto submit;

> +			ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
> +			spin_lock_irqsave(&tp->rx_lock, lockflags);
> +			if (ret && ret != -ENODEV) {
> +				list_add_tail(&agg->list, next);
> +				tasklet_schedule(&tp->tl);
> +			}
> +			continue;
> +		}

(remove the line above)

[...]
> +			rx_data = rx_agg_align(rx_data + pkt_len + 4);
> +			rx_desc = (struct rx_desc *)rx_data;
> +			pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
> +			len_used = (int)(rx_data - (u8 *)agg->head);
> +			len_used += sizeof(struct rx_desc) + pkt_len;
> +		}
> +

submit:

> +		ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
> +		spin_lock_irqsave(&tp->rx_lock, lockflags);
> +		if (ret && ret != -ENODEV) {
> +			list_add_tail(&agg->list, next);
> +			tasklet_schedule(&tp->tl);
> +		}
> +	}
> +	spin_unlock_irqrestore(&tp->rx_lock, lockflags);
> +}

It should be possible to retrieve more items in the spinlocked section
so as to have a chance to batch more work. I have not thought too deeply
about it.

> +
> +static void tx_bottom(struct r8152 *tp)
> +{
> +	struct net_device_stats *stats;
> +	struct net_device *netdev;
> +	struct tx_agg *agg;
> +	unsigned long lockflags;
> +	u32 remain, total;
> +	u8 *tx_data;
> +	int res;
> +
> +	netdev = tp->netdev;
> +
> +next_agg:

Use a real for / while loop and split this function as you experience
line length problem.

[...]
> +static void bottom_half(unsigned long data)
>  {
>  	struct r8152 *tp;
> -	int status = urb->status;
>  
> -	tp = urb->context;
> -	if (!tp)
> +	tp = (struct r8152 *)data;

  	struct r8152 *tp = (struct r8152 *)data;

[...]
> -	if (!netif_device_present(tp->netdev))
> +
> +	if (!netif_carrier_ok(tp->netdev))
>  		return;

It deserves an explanation.

[...]
> @@ -923,33 +1227,44 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
>  {
[...]
> +	spin_lock_irqsave(&tp->tx_lock, lockflags);
> +	if (!list_empty(&tp->tx_free) && skb_queue_empty(&tp->tx_queue)) {
> +		struct list_head *cursor;
> +
> +		cursor = tp->tx_free.next;
> +		list_del_init(cursor);
> +		agg = list_entry(cursor, struct tx_agg, list);

Duplicated in tx_bottom.

[...]
> @@ -999,17 +1313,18 @@ static inline u8 rtl8152_get_speed(struct r8152 *tp)
>  
>  static int rtl8152_enable(struct r8152 *tp)
>  {
> -	u32	ocp_data;
> +	u32 ocp_data;
> +	int i, ret;
>  	u8 speed;
>  
>  	speed = rtl8152_get_speed(tp);
> -	if (speed & _100bps) {
> +	if (speed & _10bps) {
>  		ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEEP_CR);
> -		ocp_data &= ~EEEP_CR_EEEP_TX;
> +		ocp_data |= EEEP_CR_EEEP_TX;
>  		ocp_write_word(tp, MCU_TYPE_PLA, PLA_EEEP_CR, ocp_data);
>  	} else {
>  		ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEEP_CR);
> -		ocp_data |= EEEP_CR_EEEP_TX;
> +		ocp_data &= ~EEEP_CR_EEEP_TX;

Call me "Mickey Mouse" if this is related to the subject of the patch.

[...]
> @@ -1631,10 +1965,12 @@ static int rtl8152_probe(struct usb_interface *intf,
>  		return -ENOMEM;
>  	}
>  
> +	SET_NETDEV_DEV(netdev, &intf->dev);
>  	tp = netdev_priv(netdev);
> +	memset(tp, 0, sizeof(*tp));

Useless, see kzalloc in net/core/dev.c::alloc_netdev_mqs

-- 
Ueimor

^ permalink raw reply

* [patch] tun: signedness bug in tun_get_user()
From: Dan Carpenter @ 2013-08-15 12:52 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jason Wang, Michael S. Tsirkin, Eric Dumazet, Neil Horman, netdev,
	kernel-janitors

The recent fix d9bf5f1309 "tun: compare with 0 instead of total_len" is
not totally correct.  Because "len" and "sizeof()" are size_t type, that
means they are never less than zero.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index af987f0..7ed13cc 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -977,8 +977,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	u32 rxhash;
 
 	if (!(tun->flags & TUN_NO_PI)) {
-		if ((len -= sizeof(pi)) < 0)
+		if (len < sizeof(pi))
 			return -EINVAL;
+		len -= sizeof(pi);
 
 		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
 			return -EFAULT;
@@ -986,8 +987,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	}
 
 	if (tun->flags & TUN_VNET_HDR) {
-		if ((len -= tun->vnet_hdr_sz) < 0)
+		if (len < tun->vnet_hdr_sz)
 			return -EINVAL;
+		len -= tun->vnet_hdr_sz;
 
 		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
 			return -EFAULT;

^ permalink raw reply related

* [PATCH v3 1/7] net: fsl_pq_mdio: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, LKML, Li Zefan, Sergei Shtylyov


Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
so we can directly pass a struct platform_device.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/freescale/fsl_pq_mdio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fsl_pq_mdio.c b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
index c93a056..b7dcd41 100644
--- a/drivers/net/ethernet/freescale/fsl_pq_mdio.c
+++ b/drivers/net/ethernet/freescale/fsl_pq_mdio.c
@@ -409,7 +409,7 @@ static int fsl_pq_mdio_probe(struct platform_device *pdev)
 	priv->regs = priv->map + data->mii_offset;

 	new_bus->parent = &pdev->dev;
-	dev_set_drvdata(&pdev->dev, new_bus);
+	platform_set_drvdata(pdev, new_bus);

 	if (data->get_tbipa) {
 		for_each_child_of_node(np, tbi) {
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 0/7] net: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Li Zefan, leoli, linuxppc-dev, pantelis.antoniou, vbordug,
	Greg KH, jg1.han, Sergei Shtylyov

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
so we can directly pass a struct platform_device.

changelog v3:
	remove modify about dev_set_drvdata()
changelog v2:
	this version add modify record about dev_set_drvdata().

Libo Chen (7):
  net: fsl_pq_mdio: use platform_{get,set}_drvdata()
  net: ucc_geth: use platform_{get,set}_drvdata()
  net: fec_mpc52xx_phy: use platform_{get,set}_drvdata()
  net: sunbmac: use platform_{get,set}_drvdata()
  net: sunhme: use platform_{get,set}_drvdata()
  net: xilinx_emaclite: use platform_{get,set}_drvdata()
  net: davinci_mdio: use platform_{get,set}_drvdata()

 drivers/net/ethernet/freescale/fec_mpc52xx_phy.c |    3 +--
 drivers/net/ethernet/freescale/fsl_pq_mdio.c     |    2 +-
 drivers/net/ethernet/freescale/ucc_geth.c        |    3 +--
 drivers/net/ethernet/sun/sunbmac.c               |    2 +-
 drivers/net/ethernet/sun/sunhme.c                |    6 +++---
 drivers/net/ethernet/ti/davinci_mdio.c           |    3 +--
 drivers/net/ethernet/xilinx/xilinx_emaclite.c    |    3 +--
 7 files changed, 9 insertions(+), 13 deletions(-)

^ permalink raw reply

* [PATCH v3 3/7] net: fec_mpc52xx_phy: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, LKML, Li Zefan, Sergei Shtylyov

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &of->dev,
so we can directly pass a struct platform_device.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/freescale/fec_mpc52xx_phy.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c b/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
index 360a578..eb44797 100644
--- a/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
+++ b/drivers/net/ethernet/freescale/fec_mpc52xx_phy.c
@@ -123,8 +123,7 @@ static int mpc52xx_fec_mdio_probe(struct platform_device *of)

 static int mpc52xx_fec_mdio_remove(struct platform_device *of)
 {
-	struct device *dev = &of->dev;
-	struct mii_bus *bus = dev_get_drvdata(dev);
+	struct mii_bus *bus = platform_get_drvdata(of);
 	struct mpc52xx_fec_mdio_priv *priv = bus->priv;

 	mdiobus_unregister(bus);
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 4/7] net: sunbmac: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller; +Cc: emilio, Greg KH, netdev, LKML, Li Zefan, Sergei Shtylyov

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &op->dev,
so we can directly pass a struct platform_device.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/sun/sunbmac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunbmac.c b/drivers/net/ethernet/sun/sunbmac.c
index 0d43fa9..0a32ca5 100644
--- a/drivers/net/ethernet/sun/sunbmac.c
+++ b/drivers/net/ethernet/sun/sunbmac.c
@@ -1239,7 +1239,7 @@ static int bigmac_sbus_probe(struct platform_device *op)

 static int bigmac_sbus_remove(struct platform_device *op)
 {
-	struct bigmac *bp = dev_get_drvdata(&op->dev);
+	struct bigmac *bp = platform_get_drvdata(op);
 	struct device *parent = op->dev.parent;
 	struct net_device *net_dev = bp->dev;
 	struct platform_device *qec_op;
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 5/7] net: sunhme: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller; +Cc: jg1.han, Bill Pemberton, netdev, LKML, Sergei Shtylyov

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
so we can directly pass a struct platform_device.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/sun/sunhme.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index 171f5b0..4900ea8 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -3111,7 +3111,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev,
 		goto err_out_iounmap;
 	}

-	dev_set_drvdata(&pdev->dev, hp);
+	platform_set_drvdata(pdev, hp);

 	if (!qfe_slot) {
 		struct pci_dev *qpdev = qp->quattro_dev;
@@ -3159,7 +3159,7 @@ err_out:

 static void happy_meal_pci_remove(struct pci_dev *pdev)
 {
-	struct happy_meal *hp = dev_get_drvdata(&pdev->dev);
+	struct happy_meal *hp = platform_get_drvdata(pdev);
 	struct net_device *net_dev = hp->dev;

 	unregister_netdev(net_dev);
@@ -3231,7 +3231,7 @@ static int hme_sbus_probe(struct platform_device *op)

 static int hme_sbus_remove(struct platform_device *op)
 {
-	struct happy_meal *hp = dev_get_drvdata(&op->dev);
+	struct happy_meal *hp = platform_get_drvdata(op);
 	struct net_device *net_dev = hp->dev;

 	unregister_netdev(net_dev);
-- 
1.7.1

^ permalink raw reply related

* [PATCH v3 2/7] net: ucc_geth: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller, leoli; +Cc: netdev, Li Zefan, Sergei Shtylyov, linuxppc-dev

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &ofdev->dev,
so we can directly pass a struct platform_device.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/freescale/ucc_geth.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 3c43dac..533885c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3911,8 +3911,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)

 static int ucc_geth_remove(struct platform_device* ofdev)
 {
-	struct device *device = &ofdev->dev;
-	struct net_device *dev = dev_get_drvdata(device);
+	struct net_device *dev = platform_get_drvdata(ofdev);
 	struct ucc_geth_private *ugeth = netdev_priv(dev);

 	unregister_netdev(dev);
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 0/5] qlcnic: Fixes for Mailbox command handling.
From: Sucheta Chakraborty @ 2013-08-15 12:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev

All the bug fixes in this series are dependent on net-next patches which were
submitted previously by commit (qlcnic: Interrupt based driver firmware mailbox
mechanism, commit id - e5c4e6c696aea58fbea5758e8b2841d2b0309cf7).

Please apply this series to net-next.

Thanks,
Sucheta.

Manish Chopra (4):
  qlcnic: Reinitialize mailbox data structures after firmware reset
  qlcnic: Flush mailbox command list when mailbox is not available
  qlcnic: Fix driver initialization for 83xx adapters
  qlcnic: Dump mailbox command data when a command times out

Sucheta Chakraborty (1):
  qlcnic: Update version to 5.2.46.

 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h              |  4 ++--
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c      |  8 +++++++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c    | 14 +++++++-------
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c |  2 +-
 4 files changed, 17 insertions(+), 11 deletions(-)

^ permalink raw reply

* [PATCH v3 7/7] net: davinci_mdio: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-15 13:01 UTC (permalink / raw)
  To: David Miller
  Cc: mugunthanvnm, bigeasy, prabhakar.csengg, b-liu, netdev, LKML,
	Li Zefan, Sergei Shtylyov

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &pdev->dev,
so we can directly pass a struct platform_device.

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/ti/davinci_mdio.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 16ddfc3..01b0cc5 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -421,8 +421,7 @@ bail_out:

 static int davinci_mdio_remove(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
-	struct davinci_mdio_data *data = dev_get_drvdata(dev);
+	struct davinci_mdio_data *data = platform_get_drvdata(pdev);

 	if (data->bus) {
 		mdiobus_unregister(data->bus);
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 3/5] qlcnic: Fix driver initialization for 83xx adapters
From: Sucheta Chakraborty @ 2013-08-15 12:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev, Manish Chopra
In-Reply-To: <1376569649-565-1-git-send-email-sucheta.chakraborty@qlogic.com>

From: Manish Chopra <manish.chopra@qlogic.com>

o Load firmware from file before setting up interrupts.

Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
index cc1e32a..17c26a1 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
@@ -2169,6 +2169,13 @@ int qlcnic_83xx_init(struct qlcnic_adapter *adapter, int pci_using_dac)
 	if (err)
 		goto detach_mbx;
 
+	if (!qlcnic_83xx_read_flash_descriptor_table(adapter))
+		qlcnic_83xx_read_flash_mfg_id(adapter);
+
+	err = qlcnic_83xx_idc_init(adapter);
+	if (err)
+		goto detach_mbx;
+
 	err = qlcnic_setup_intr(adapter, 0);
 	if (err) {
 		dev_err(&adapter->pdev->dev, "Failed to setup interrupt\n");
@@ -2186,13 +2193,6 @@ int qlcnic_83xx_init(struct qlcnic_adapter *adapter, int pci_using_dac)
 	/* register for NIC IDC AEN Events */
 	qlcnic_83xx_register_nic_idc_func(adapter, 1);
 
-	if (!qlcnic_83xx_read_flash_descriptor_table(adapter))
-		qlcnic_83xx_read_flash_mfg_id(adapter);
-
-	err = qlcnic_83xx_idc_init(adapter);
-	if (err)
-		goto disable_mbx_intr;
-
 	/* Configure default, SR-IOV or Virtual NIC mode of operation */
 	err = qlcnic_83xx_configure_opmode(adapter);
 	if (err)
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 1/3] qlcnic: Fix set driver version command
From: Sucheta Chakraborty @ 2013-08-15 12:29 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev, Himanshu Madhani
In-Reply-To: <1376569769-625-1-git-send-email-sucheta.chakraborty@qlogic.com>

From: Himanshu Madhani <himanshu.madhani@qlogic.com>

Driver was issuing set driver version command through all
functions in the adapter. Fix the driver to issue set driver
version once per adapter, through function 0.

Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c | 3 ++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c      | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
index 9f4b8d5..345d987 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
@@ -629,7 +629,8 @@ int qlcnic_83xx_idc_reattach_driver(struct qlcnic_adapter *adapter)
 		return -EIO;
 	}
 
-	qlcnic_set_drv_version(adapter);
+	if (adapter->portnum == 0)
+		qlcnic_set_drv_version(adapter);
 	qlcnic_83xx_idc_attach_driver(adapter);
 
 	return 0;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index ee013fc..bc05d01 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2165,7 +2165,8 @@ qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_out_disable_mbx_intr;
 
-	qlcnic_set_drv_version(adapter);
+	if (adapter->portnum == 0)
+		qlcnic_set_drv_version(adapter);
 
 	pci_set_drvdata(pdev, adapter);
 
@@ -3085,7 +3086,8 @@ done:
 	adapter->fw_fail_cnt = 0;
 	adapter->flags &= ~QLCNIC_FW_HANG;
 	clear_bit(__QLCNIC_RESETTING, &adapter->state);
-	qlcnic_set_drv_version(adapter);
+	if (adapter->portnum == 0)
+		qlcnic_set_drv_version(adapter);
 
 	if (!qlcnic_clr_drv_state(adapter))
 		qlcnic_schedule_work(adapter, qlcnic_fw_poll_work,
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 2/3] qlcnic: Fix beacon state return status handling
From: Sucheta Chakraborty @ 2013-08-15 12:29 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev
In-Reply-To: <1376569769-625-1-git-send-email-sucheta.chakraborty@qlogic.com>

o Driver was misinterpreting the return status for beacon
  state query leading to incorrect interpretation of beacon
  state and logging an error message for successful status.
  Fixed the driver to properly interpret the return status.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index 10ed82b..660c3f5 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -170,9 +170,9 @@ static int qlcnic_82xx_store_beacon(struct qlcnic_adapter *adapter,
 
 	if (ahw->extra_capability[0] & QLCNIC_FW_CAPABILITY_2_BEACON) {
 		err = qlcnic_get_beacon_state(adapter, &h_beacon_state);
-		if (!err) {
-			dev_info(&adapter->pdev->dev,
-				 "Failed to get current beacon state\n");
+		if (err) {
+			netdev_err(adapter->netdev,
+				   "Failed to get current beacon state\n");
 		} else {
 			if (h_beacon_state == QLCNIC_BEACON_DISABLE)
 				ahw->beacon_state = 0;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 0/3] qlcnic: bug fixes
From: Sucheta Chakraborty @ 2013-08-15 12:29 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev

Please apply to net.

Thanks,
Sucheta.

Himanshu Madhani (1):
  qlcnic: Fix set driver version command

Manish Chopra (1):
  qlcnic: Fix diagnostic interrupt test for 83xx adapters

Sucheta Chakraborty (1):
  qlcnic: Fix beacon state return status handling

 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c   | 5 +++++
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c | 3 ++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c      | 6 ++++--
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c     | 6 +++---
 4 files changed, 14 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/5] qlcnic: Reinitialize mailbox data structures after firmware reset
From: Sucheta Chakraborty @ 2013-08-15 12:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev, Manish Chopra
In-Reply-To: <1376569649-565-1-git-send-email-sucheta.chakraborty@qlogic.com>

From: Manish Chopra <manish.chopra@qlogic.com>

o After firmware reset VFs were failing to come up because of not
  reinitializing mailbox data structures. Reinitialize them so that
  VFs can come up after firmware reset.

Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c
index 046286a..dc24979 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c
@@ -1556,7 +1556,7 @@ static int qlcnic_sriov_vf_reinit_driver(struct qlcnic_adapter *adapter)
 {
 	int err;
 
-	set_bit(QLC_83XX_MBX_READY, &adapter->ahw->idc.status);
+	qlcnic_83xx_reinit_mbx_work(adapter->ahw->mailbox);
 	qlcnic_83xx_enable_mbx_interrupt(adapter);
 
 	err = qlcnic_sriov_cfg_bc_intr(adapter, 1);
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 2/5] qlcnic: Flush mailbox command list when mailbox is not available
From: Sucheta Chakraborty @ 2013-08-15 12:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev, Manish Chopra
In-Reply-To: <1376569649-565-1-git-send-email-sucheta.chakraborty@qlogic.com>

From: Manish Chopra <manish.chopra@qlogic.com>

o Driver was hitting a panic at the time of adapter reset due to invalid command
  access from the list which had been already freed by the queuing thread.
  Flush all the pending commands from the list before proceeding with adapter reset

Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 55a5977..43b51a0 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -3515,6 +3515,8 @@ static inline void qlcnic_83xx_flush_mbx_queue(struct qlcnic_adapter *adapter)
 
 	while (!list_empty(head)) {
 		cmd = list_entry(head->next, struct qlcnic_cmd_args, list);
+		dev_info(&adapter->pdev->dev, "%s: Mailbox command 0x%x\n",
+			 __func__, cmd->cmd_op);
 		list_del(&cmd->list);
 		mbx->num_cmds--;
 		qlcnic_83xx_notify_cmd_completion(adapter, cmd);
@@ -3534,6 +3536,7 @@ static inline int qlcnic_83xx_check_mbx_status(struct qlcnic_adapter *adapter)
 
 	host_mbx_ctrl = QLCRDX(ahw, QLCNIC_HOST_MBX_CTRL);
 	if (host_mbx_ctrl) {
+		clear_bit(QLC_83XX_MBX_READY, &mbx->status);
 		ahw->idc.collect_dump = 1;
 		return -EIO;
 	}
@@ -3704,8 +3707,10 @@ static void qlcnic_83xx_mailbox_worker(struct work_struct *work)
 	ahw = adapter->ahw;
 
 	while (true) {
-		if (qlcnic_83xx_check_mbx_status(adapter))
+		if (qlcnic_83xx_check_mbx_status(adapter)) {
+			qlcnic_83xx_flush_mbx_queue(adapter);
 			return;
+		}
 
 		atomic_set(rsp_status, QLC_83XX_MBX_RESPONSE_WAIT);
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 5/5] qlcnic: Update version to 5.2.46.
From: Sucheta Chakraborty @ 2013-08-15 12:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev
In-Reply-To: <1376569649-565-1-git-send-email-sucheta.chakraborty@qlogic.com>

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 3935155..7387354 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -37,8 +37,8 @@
 
 #define _QLCNIC_LINUX_MAJOR 5
 #define _QLCNIC_LINUX_MINOR 2
-#define _QLCNIC_LINUX_SUBVERSION 45
-#define QLCNIC_LINUX_VERSIONID  "5.2.45"
+#define _QLCNIC_LINUX_SUBVERSION 46
+#define QLCNIC_LINUX_VERSIONID  "5.2.46"
 #define QLCNIC_DRV_IDC_VER  0x01
 #define QLCNIC_DRIVER_VERSION  ((_QLCNIC_LINUX_MAJOR << 16) |\
 		 (_QLCNIC_LINUX_MINOR << 8) | (_QLCNIC_LINUX_SUBVERSION))
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net 3/3] qlcnic: Fix diagnostic interrupt test for 83xx adapters
From: Sucheta Chakraborty @ 2013-08-15 12:29 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev, Manish Chopra
In-Reply-To: <1376569769-625-1-git-send-email-sucheta.chakraborty@qlogic.com>

From: Manish Chopra <manish.chopra@qlogic.com>

o Do not allow interrupt test when adapter is resetting.

Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 92da998..9d4bb7f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -3266,6 +3266,11 @@ int qlcnic_83xx_interrupt_test(struct net_device *netdev)
 	u8 val;
 	int ret, max_sds_rings = adapter->max_sds_rings;
 
+	if (test_bit(__QLCNIC_RESETTING, &adapter->state)) {
+		netdev_info(netdev, "Device is resetting\n");
+		return -EBUSY;
+	}
+
 	if (qlcnic_get_diag_lock(adapter)) {
 		netdev_info(netdev, "Device in diagnostics mode\n");
 		return -EBUSY;
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next 4/5] qlcnic: Dump mailbox command data when a command times out
From: Sucheta Chakraborty @ 2013-08-15 12:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept-HSGLinuxNICDev, Manish Chopra
In-Reply-To: <1376569649-565-1-git-send-email-sucheta.chakraborty@qlogic.com>

From: Manish Chopra <manish.chopra@qlogic.com>

Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
index 43b51a0..78ca755 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
@@ -3736,6 +3736,7 @@ static void qlcnic_83xx_mailbox_worker(struct work_struct *work)
 				__func__, cmd->cmd_op, cmd->type, ahw->pci_func,
 				ahw->op_mode);
 			clear_bit(QLC_83XX_MBX_READY, &mbx->status);
+			qlcnic_dump_mbx(adapter, cmd);
 			qlcnic_83xx_idc_request_reset(adapter,
 						      QLCNIC_FORCE_FW_DUMP_KEY);
 			cmd->rsp_opcode = QLCNIC_RCODE_TIMEOUT;
-- 
1.8.1.4

^ permalink raw reply related

* Re: [patch] tun: signedness bug in tun_get_user()
From: Michael S. Tsirkin @ 2013-08-15 14:04 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
	kernel-janitors
In-Reply-To: <20130815125257.GA16932@elgon.mountain>

On Thu, Aug 15, 2013 at 03:52:57PM +0300, Dan Carpenter wrote:
> The recent fix d9bf5f1309 "tun: compare with 0 instead of total_len" is
> not totally correct.  Because "len" and "sizeof()" are size_t type, that
> means they are never less than zero.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index af987f0..7ed13cc 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -977,8 +977,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	u32 rxhash;
>  
>  	if (!(tun->flags & TUN_NO_PI)) {
> -		if ((len -= sizeof(pi)) < 0)
> +		if (len < sizeof(pi))
>  			return -EINVAL;
> +		len -= sizeof(pi);
>  
>  		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
>  			return -EFAULT;
> @@ -986,8 +987,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	}
>  
>  	if (tun->flags & TUN_VNET_HDR) {
> -		if ((len -= tun->vnet_hdr_sz) < 0)
> +		if (len < tun->vnet_hdr_sz)
>  			return -EINVAL;
> +		len -= tun->vnet_hdr_sz;
>  
>  		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
>  			return -EFAULT;

^ permalink raw reply

* Re: [patch] tun: signedness bug in tun_get_user()
From: Michael S. Tsirkin @ 2013-08-15 14:58 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
	kernel-janitors
In-Reply-To: <20130815140449.GA8064@redhat.com>

On Thu, Aug 15, 2013 at 05:04:49PM +0300, Michael S. Tsirkin wrote:
> On Thu, Aug 15, 2013 at 03:52:57PM +0300, Dan Carpenter wrote:
> > The recent fix d9bf5f1309 "tun: compare with 0 instead of total_len" is
> > not totally correct.  Because "len" and "sizeof()" are size_t type, that
> > means they are never less than zero.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Alternatively how about we revert the original patch?
This is not the only issue it introduced and it doesn't
actually fix any bugs.

> > 
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index af987f0..7ed13cc 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -977,8 +977,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >  	u32 rxhash;
> >  
> >  	if (!(tun->flags & TUN_NO_PI)) {
> > -		if ((len -= sizeof(pi)) < 0)
> > +		if (len < sizeof(pi))
> >  			return -EINVAL;
> > +		len -= sizeof(pi);
> >  
> >  		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
> >  			return -EFAULT;
> > @@ -986,8 +987,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> >  	}
> >  
> >  	if (tun->flags & TUN_VNET_HDR) {
> > -		if ((len -= tun->vnet_hdr_sz) < 0)
> > +		if (len < tun->vnet_hdr_sz)
> >  			return -EINVAL;
> > +		len -= tun->vnet_hdr_sz;
> >  
> >  		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> >  			return -EFAULT;

^ permalink raw reply

* Re: [patch] tun: signedness bug in tun_get_user()
From: Michael S. Tsirkin @ 2013-08-15 15:02 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
	kernel-janitors
In-Reply-To: <20130815145840.GA8889@redhat.com>

On Thu, Aug 15, 2013 at 05:58:40PM +0300, Michael S. Tsirkin wrote:
> On Thu, Aug 15, 2013 at 05:04:49PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Aug 15, 2013 at 03:52:57PM +0300, Dan Carpenter wrote:
> > > The recent fix d9bf5f1309 "tun: compare with 0 instead of total_len" is
> > > not totally correct.  Because "len" and "sizeof()" are size_t type, that
> > > means they are never less than zero.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Alternatively how about we revert the original patch?
> This is not the only issue it introduced and it doesn't
> actually fix any bugs.
> 
> > > 
> > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > index af987f0..7ed13cc 100644
> > > --- a/drivers/net/tun.c
> > > +++ b/drivers/net/tun.c
> > > @@ -977,8 +977,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> > >  	u32 rxhash;
> > >  
> > >  	if (!(tun->flags & TUN_NO_PI)) {
> > > -		if ((len -= sizeof(pi)) < 0)
> > > +		if (len < sizeof(pi))
> > >  			return -EINVAL;
> > > +		len -= sizeof(pi);
> > >  
> > >  		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
> > >  			return -EFAULT;
> > > @@ -986,8 +987,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> > >  	}
> > >  
> > >  	if (tun->flags & TUN_VNET_HDR) {
> > > -		if ((len -= tun->vnet_hdr_sz) < 0)
> > > +		if (len < tun->vnet_hdr_sz)
> > >  			return -EINVAL;

And to be even more explicit, this still doesn't handle the
case vnet_hdr_sz < 0 properly.


> > > +		len -= tun->vnet_hdr_sz;
> > >  
> > >  		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> > >  			return -EFAULT;

^ permalink raw reply

* Re: [patch] tun: signedness bug in tun_get_user()
From: Michael S. Tsirkin @ 2013-08-15 15:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
	kernel-janitors
In-Reply-To: <20130815150214.GB8889@redhat.com>

On Thu, Aug 15, 2013 at 06:02:14PM +0300, Michael S. Tsirkin wrote:
> On Thu, Aug 15, 2013 at 05:58:40PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Aug 15, 2013 at 05:04:49PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Aug 15, 2013 at 03:52:57PM +0300, Dan Carpenter wrote:
> > > > The recent fix d9bf5f1309 "tun: compare with 0 instead of total_len" is
> > > > not totally correct.  Because "len" and "sizeof()" are size_t type, that
> > > > means they are never less than zero.
> > > > 
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > 
> > > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Alternatively how about we revert the original patch?
> > This is not the only issue it introduced and it doesn't
> > actually fix any bugs.
> > 
> > > > 
> > > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > > index af987f0..7ed13cc 100644
> > > > --- a/drivers/net/tun.c
> > > > +++ b/drivers/net/tun.c
> > > > @@ -977,8 +977,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> > > >  	u32 rxhash;
> > > >  
> > > >  	if (!(tun->flags & TUN_NO_PI)) {
> > > > -		if ((len -= sizeof(pi)) < 0)
> > > > +		if (len < sizeof(pi))
> > > >  			return -EINVAL;
> > > > +		len -= sizeof(pi);
> > > >  
> > > >  		if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi)))
> > > >  			return -EFAULT;
> > > > @@ -986,8 +987,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> > > >  	}
> > > >  
> > > >  	if (tun->flags & TUN_VNET_HDR) {
> > > > -		if ((len -= tun->vnet_hdr_sz) < 0)
> > > > +		if (len < tun->vnet_hdr_sz)
> > > >  			return -EINVAL;
> 
> And to be even more explicit, this still doesn't handle the
> case vnet_hdr_sz < 0 properly.

Hmm ENOCOFFEE.
User can't make vnet_hdr_sz < 0 - we already catch that.
So let's apply Dan's patch, it does fix all issues after all.
Sorry about the noise.

> 
> > > > +		len -= tun->vnet_hdr_sz;
> > > >  
> > > >  		if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> > > >  			return -EFAULT;

^ permalink raw reply

* Re: [PATCH net] net: tg3: fix NULL pointer dereference in tg3_io_error_detected and tg3_io_slot_reset
From: Michael Chan @ 2013-08-15 15:15 UTC (permalink / raw)
  To: David Miller; +Cc: nsujir, netdev, dborkman, shangw
In-Reply-To: <20130815.010726.719593168454341579.davem@davemloft.net>

On Thu, 2013-08-15 at 01:07 -0700, David Miller wrote: 
> From: "Nithin Nayak Sujir" <nsujir@broadcom.com>
> Date: Tue, 13 Aug 2013 11:45:13 -0700
> 
> > From: Daniel Borkmann <dborkman@redhat.com>
> > 
> > Commit d8af4dfd8 ("net/tg3: Fix kernel crash") introduced a possible
> > NULL pointer dereference in tg3 driver when !netdev || !netif_running(netdev)
> > condition is met and netdev is NULL. Then, the jump to the 'done' label
> > calls dev_close() with a netdevice that is NULL. Therefore, only call
> > dev_close() when we have a netdevice, but one that is not running.
> > 
> > [ Add the same checks in tg3_io_slot_reset() per Gavin Shan - by Nithin
> > Nayak Sujir ]
> > 
> > Reported-by: Dave Jones <davej@redhat.com>
> > Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> > Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
> > Cc: Michael Chan <mchan@broadcom.com>
> > Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
> 
> Can I get some reviews from the Broadcom folks?
> 

We worked with Daniel at Red Hat and Gavin at IBM on this patch.
Thanks.

Signed-off-by: Michael Chan <mchan@broadcom.com>

^ permalink raw reply

* Re: [PATCH net] skge: dma_sync the whole receive buffer
From: Stephen Hemminger @ 2013-08-15 15:41 UTC (permalink / raw)
  To: poma; +Cc: David Miller, netdev
In-Reply-To: <520BCC72.8040002@gmail.com>

On Wed, 14 Aug 2013 20:29:06 +0200
poma <pomidorabelisima@gmail.com> wrote:

> On 14.08.2013 18:20, Stephen Hemminger wrote:
> > On Wed, 14 Aug 2013 12:20:03 +0200
> > poma <pomidorabelisima@gmail.com> wrote:
> > 
> >> On 14.08.2013 03:00, Stephen Hemminger wrote:
> >>> On Tue, 13 Aug 2013 15:09:55 -0700 (PDT)
> >>> David Miller <davem@davemloft.net> wrote:
> >>>
> >>>> From: Stephen Hemminger <stephen@networkplumber.org>
> >>>> Date: Sat, 10 Aug 2013 15:02:07 -0700
> >>>>
> >>>>> The DMA sync should sync the whole receive buffer, not just
> >>>>> part of it. Fixes log messages dma_sync_check.
> >>>>>
> >>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>>
> >>>> Applied, but I really suspect that your "check DMA mapping errors"
> >>>> patch has added a serious regression.  A regression much worse than
> >>>> the bug you were trying to fix with that change.
> >>>
> >>> Argh. The problem is deeper than that. Device got broken somewhere between
> >>> 3.2 and 3.4. My old Dlink card works on 3.2 but gets DMA errors on 3.4.
> >>> The config's are different though so checking that as well.
> >>>
> >>
> >> Can I help you with debugging?
> >> DGE-530T is rather solid device.
> > 
> > Don't think it is a hardware problem.
> > The failure is when the board access the Receive ring PCI memory area.
> > This region is allocated with pci_alloc_consistent and therefore should
> > be available. Two possible issues are driver math issues, or hardware
> > problems with where the region is located. Some of these cards don't
> > really have full 64 bit PCI support.
> > 
> > My board is:
> > 05:01.0 Ethernet controller: D-Link System Inc Gigabit Ethernet Adapter (rev 11)
> > 	Subsystem: D-Link System Inc DGE-530T Gigabit Ethernet Adapter
> > 	Flags: bus master, 66MHz, medium devsel, latency 32, IRQ 18
> > 	Memory at f7d20000 (32-bit, non-prefetchable) [size=16K]
> > 	I/O ports at c000 [size=256]
> > 	Expansion ROM at f7d00000 [disabled] [size=128K]
> > 	Capabilities: [48] Power Management version 2
> > 	Capabilities: [50] Vital Product Data
> > 	Kernel driver in use: skge
> > 
> > 
> > What is your config?
> > 
> 
> 01:09.0 Ethernet controller: D-Link System Inc Gigabit Ethernet Adapter
> (rev 11)
> 	Subsystem: D-Link System Inc DGE-530T Gigabit Ethernet Adapter
> 	Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 19
> 	Memory at fbffc000 (32-bit, non-prefetchable) [size=16K]
> 	I/O ports at b400 [size=256]
> 	[virtual] Expansion ROM at ec000000 [disabled] [size=128K]
> 	Capabilities: [48] Power Management version 2
> 	Capabilities: [50] Vital Product Data
> 	Kernel driver in use: skge
> 
> 
> poma
> 

In the course of debugging this, I moved the card to another slot
and all the problems went away. I suspect either card insertion or more likely
the crap consumer motherboards don't have full PCI support on some slots.

There doesn't seem to be anyway to address this in software.

^ permalink raw reply


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