Re: [Patch net] nfc: check sock state in llcp_sock_getname()

[Dmitry Vyukov]

On Sat, Jan 2, 2016 at 1:34 AM, Cong Wang  wrote:
> llcp_sock_getname() checks llcp_sock->dev to make sure
> llcp_sock is already connected or bound, however, we could
> be in the middle of llcp_sock_bind() where llcp_sock->dev
> is bound and llcp_sock->service_name_len is set,
> but llcp_sock->service_name is not, in this case we would
> lead to copy some bytes from a NULL pointer.
>
> We should just check if sk->sk_state is still closed since
> both connect() and bind() will update this state at the end.

Hi Cong,

This is still racy. If you want to play lock-free then you also need
proper memory barriers. Stores to sk_state need to be
smp_store_release, while the load needs to be smp_load_acquire.
Otherwise getname still can see partially initialized socket.


> Reported-by: Dmitry Vyukov 
> Cc: Lauro Ramos Venancio 
> Cc: Aloisio Almeida Jr 
> Cc: Samuel Ortiz 
> Signed-off-by: Cong Wang 
> ---
>  net/nfc/llcp_sock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
> index ecf0a01..5a91997 100644
> --- a/net/nfc/llcp_sock.c
> +++ b/net/nfc/llcp_sock.c
> @@ -500,7 +500,7 @@ static int llcp_sock_getname(struct socket *sock, struct 
> sockaddr *uaddr,
> struct nfc_llcp_sock *llcp_sock = nfc_llcp_sock(sk);
> DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, llcp_addr, uaddr);
>
> -   if (llcp_sock == NULL || llcp_sock->dev == NULL)
> +   if (llcp_sock == NULL || sk->sk_state == LLCP_CLOSED)
> return -EBADFD;
>
> pr_debug("%p %d %d %d\n", sk, llcp_sock->target_idx,
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] phy: add HAS_IOMEM/OF_MDIO dependencies to MDIO_OCTEON

[Vegard Nossum]

Ran into this on UML:

drivers/built-in.o: In function `octeon_mdiobus_probe':
drivers/net/phy/mdio-octeon.c:295: undefined reference to `devm_ioremap'
drivers/net/phy/mdio-octeon.c:320: undefined reference to `of_mdiobus_register'
collect2: error: ld returned 1 exit status

devm_ioremap() is defined only when HAS_IOMEM is selected.

of_mdiobus_register() is defined only when OF_MDIO is selected.

Signed-off-by: Vegard Nossum 
Cc: Florian Fainelli 
Cc: netdev@vger.kernel.org
---
 drivers/net/phy/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 60994a8..19314bb 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -186,6 +186,8 @@ config MDIO_GPIO
 config MDIO_OCTEON
tristate "Support for MDIO buses on Octeon and ThunderX SOCs"
depends on 64BIT
+   depends on HAS_IOMEM
+   depends on OF_MDIO
help
 
  This module provides a driver for the Octeon and ThunderX MDIO
-- 
1.9.1

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

Re: [PATCH] phy: add HAS_IOMEM/OF_MDIO dependencies to MDIO_OCTEON

[kbuild test robot]

Hi Vegard,

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.4-rc7 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Vegard-Nossum/phy-add-HAS_IOMEM-OF_MDIO-dependencies-to-MDIO_OCTEON/20160102-211011
config: x86_64-randconfig-s5-01022139 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

warning: (OCTEON_MGMT_ETHERNET && THUNDER_NIC_BGX && OCTEON_ETHERNET) selects 
MDIO_OCTEON which has unmet direct dependencies (NETDEVICES && PHYLIB && 64BIT 
&& HAS_IOMEM && OF_MDIO)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

[PATCH 0/3] net-rsi: Fine-tuning for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 15:36:25 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
  Delete unnecessary variable initialisations in rsi_send_data_pkt()
  Replace variable initialisations by assignments in rsi_send_data_pkt()

 drivers/net/wireless/rsi/rsi_91x_pkt.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.6.3

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

Re: [PATCH] net-libertas: Better exception handling in if_spi_host_to_card_worker()

[SF Markus Elfring]

>> Move the jump label directly before the desired log statement
>> so that the variable "err" will not be checked once more
>> after it was determined that a function call failed.
>> Use the identifier "report_failure" instead of the label "err".
> 
>Why?

I suggest to reconsider the places with which such a jump label
is connected.


> The code was smart enough

Which action should really be performed after a failure was detected
and handled a bit already?

* Another condition check

* Just additional error logging


> and you're making it uglier that it needs to be.

I assume that a software development taste can evolve, can't it?

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

Re: net-libertas: Better exception handling in if_spi_host_to_card_worker()

[SF Markus Elfring]

> I have never seen much evolution going on in this area.

I can get an other impression from a specific document for example.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/Documentation/CodingStyle


> What the patch tries to do is avoid the extra 'if (err)'.

Yes. - I propose to look at related consequences together with the usage
of a popular short jump label once more.


> Setting coding style aside, the question is whether there is
> a good metric for the patch.

A software development challenge is to accept changes also around a topic
like "error handling". My update suggestion for the source file
"drivers/net/wireless/marvell/libertas/if_spi.c" should only improve
exception handling. (I came along other source files where more improvements
will eventually be possible.)

When will the run-time behaviour matter also for exceptional situations?


> Did you look at the resulting assembly code for different target 
> architectures?

Not yet. - Which execution system variants would you recommend for
further comparisons?

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

[PATCH 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 15:25:34 +0100

Replace explicit initialisation for two local variables at the beginning
by assignments.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index ec65e1c..fe36e7d 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -26,12 +26,12 @@
  */
 int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
 {
-   struct rsi_hw *adapter = common->priv;
+   struct rsi_hw *adapter;
struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
struct ieee80211_bss_conf *bss;
-   int status = -EINVAL;
+   int status;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
u8 extnd_size;
__le16 *frame_desc;
@@ -41,8 +41,10 @@ int rsi_send_data_pkt(struct rsi_common *common, struct 
sk_buff *skb)
bss = >control.vif->bss_conf;
tx_params = (struct skb_info *)info->driver_data;
 
-   if (!bss->assoc)
+   if (!bss->assoc) {
+   status = -EINVAL;
goto err;
+   }
 
tmp_hdr = (struct ieee80211_hdr *)>data[0];
seq_num = (le16_to_cpu(tmp_hdr->seq_ctrl) >> 4);
@@ -97,7 +99,7 @@ int rsi_send_data_pkt(struct rsi_common *common, struct 
sk_buff *skb)
frame_desc[7] = cpu_to_le16(((tx_params->tid & 0xf) << 4) |
(skb->priority & 0xf) |
(tx_params->sta_id << 8));
-
+   adapter = common->priv;
status = adapter->host_intf_write_pkt(common->priv,
  skb->data,
  skb->len);
-- 
2.6.3

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

Re: net-libertas: Better exception handling in if_spi_host_to_card_worker()

[Arend van Spriel]

On 02-01-16 10:08, SF Markus Elfring wrote:
>>> I assume that a software development taste can evolve, can't it?
>>
>> So far, you have gotten several down votes for this kind of change,
> 
> I am curious when more contributors will share corresponding opinions.

Let's burn some cycles on this while the holidays give me time to do so.
"software development taste" is another term for "coding style". In
every project battles are fought over this between friends and foes. I
have never seen much evolution going on in this area.

>> and no enthusiasm.
> 
> How many software designers and developers can become enthusiastic
> about better exception handling to some degree?

I had to  take a look at this particular patch and I have to say that I
don't see, using your favorite term, evolution at work. It looks more
like the result of inbred. What the patch tries to do is avoid the extra
'if (err)'. Setting coding style aside, the question is whether there is
a good metric for the patch. So does it really safe processing time? Did
you look at the resulting assembly code for different target architectures?

You got pushed back on the change so you have to come up with solid
arguments for your change instead of spewing ideas about evolution in
software development. Running Coccinelle is one thing, but understanding
the results and what you are ultimately proposing to be changed is more
important.

Regards,
Arend

>> The code that is performance critical, you should probably not touch, ever.
> 
> I imagine that technical evolution will result in further considerations
> so that "unchangeable" components can be adjusted once more.
> 
> 
>> The people who wrote it knew what was important and what was not.
> 
> I might come along at some places where the affected knowledge will also 
> evolve.
> 
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net-brcmfmac: Delete an unnecessary variable initialisation in brcmf_sdio_download_firmware()

[Arend van Spriel]

On 01/01/2016 08:26 PM, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Fri, 1 Jan 2016 20:20:15 +0100


I think it has been said over and over, but please use driver name only 
as prefix. I don't see value to prepend it with 'net-'.



Omit explicit initialisation at the beginning for one local variable
that is redefined before its first use.



That being said here is my

Acked-by: Arend van Spriel 

Signed-off-by: Markus Elfring 
---
  drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index ceb2a75..c21eeb1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -3260,7 +3260,7 @@ static int brcmf_sdio_download_firmware(struct brcmf_sdio 
*bus,
const struct firmware *fw,
void *nvram, u32 nvlen)
  {
-   int bcmerror = -EFAULT;
+   int bcmerror;
u32 rstvec;

sdio_claim_host(bus->sdiodev->func[1]);



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

Re: [PATCH] net-libertas: Better exception handling in if_spi_host_to_card_worker()

[Julia Lawall]

On Sat, 2 Jan 2016, SF Markus Elfring wrote:

> >> Move the jump label directly before the desired log statement
> >> so that the variable "err" will not be checked once more
> >> after it was determined that a function call failed.
> >> Use the identifier "report_failure" instead of the label "err".
> > 
> >Why?
> 
> I suggest to reconsider the places with which such a jump label
> is connected.
> 
> 
> > The code was smart enough
> 
> Which action should really be performed after a failure was detected
> and handled a bit already?
> 
> * Another condition check
> 
> * Just additional error logging
> 
> 
> > and you're making it uglier that it needs to be.
> 
> I assume that a software development taste can evolve, can't it?

So far, you have gotten several down votes for this kind of change, and no 
enthusiasm.

Admittedly, this is a trivial case, because there are no local variables, 
but do you actually know the semantics in C of a jump into a block?  And 
if you do know, do you think that this semantics is common knowledge?  And 
do you really think that introducing poorly understandable code is really 
worth saving an if test of a single variable on a non-critical path?

Most of the kernel code is not performance critical at the level of a 
single if test.  So the goal should be for the code to be easy to 
understand and robust to change.  The code that is performance critical, 
you should probably not touch, ever.  The people who wrote it knew what 
was important and what was not.

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

Re: net-libertas: Better exception handling in if_spi_host_to_card_worker()

[SF Markus Elfring]

>> I assume that a software development taste can evolve, can't it?
> 
> So far, you have gotten several down votes for this kind of change,

I am curious when more contributors will share corresponding opinions.


> and no enthusiasm.

How many software designers and developers can become enthusiastic
about better exception handling to some degree?


> The code that is performance critical, you should probably not touch, ever.

I imagine that technical evolution will result in further considerations
so that "unchangeable" components can be adjusted once more.


> The people who wrote it knew what was important and what was not.

I might come along at some places where the affected knowledge will also evolve.

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

[PATCH 2/3] rsi: Delete unnecessary variable initialisations in rsi_send_data_pkt()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 15:15:12 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index ee98f5b..ec65e1c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -27,15 +27,15 @@
 int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
 {
struct rsi_hw *adapter = common->priv;
-   struct ieee80211_hdr *tmp_hdr = NULL;
+   struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
-   struct ieee80211_bss_conf *bss = NULL;
+   struct ieee80211_bss_conf *bss;
int status = -EINVAL;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
-   u8 extnd_size = 0;
+   u8 extnd_size;
__le16 *frame_desc;
-   u16 seq_num = 0;
+   u16 seq_num;
 
info = IEEE80211_SKB_CB(skb);
bss = >control.vif->bss_conf;
-- 
2.6.3

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

[PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 14:54:30 +0100

Omit explicit initialisation at the beginning for five local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..ee98f5b 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
  struct sk_buff *skb)
 {
struct rsi_hw *adapter = common->priv;
-   struct ieee80211_hdr *wh = NULL;
+   struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
-   struct ieee80211_bss_conf *bss = NULL;
+   struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = >conf;
struct skb_info *tx_params;
-   int status = -E2BIG;
-   __le16 *msg = NULL;
-   u8 extnd_size = 0;
+   int status;
+   __le16 *msg;
+   u8 extnd_size;
u8 vap_id = 0;
 
info = IEEE80211_SKB_CB(skb);
-- 
2.6.3

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

[PATCH 1/5] xen-netback: Delete an unnecessary assignment in connect_rings()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 17:32:40 +0100

Remove the assignment for a local variable because its value is not
changed compared to the one from a previous function call.

Signed-off-by: Markus Elfring 
---
 drivers/net/xen-netback/xenbus.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 56ebd82..7f2895d 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -941,7 +941,6 @@ static int connect_rings(struct backend_info *be, struct 
xenvif_queue *queue)
goto err;
}
 
-   err = 0;
 err: /* Regular return falls through with err == 0 */
kfree(xspath);
return err;
-- 
2.6.3

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

[PATCH 2/5] xen-netback: Delete an unnecessary goto statement in connect_rings()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 17:50:21 +0100

One goto statement referred to a source code position directly behind it.
Thus omit such an unnecessary jump.

Signed-off-by: Markus Elfring 
---
 drivers/net/xen-netback/xenbus.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 7f2895d..d4947e1 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -933,13 +933,11 @@ static int connect_rings(struct backend_info *be, struct 
xenvif_queue *queue)
/* Map the shared frame, irq etc. */
err = xenvif_connect(queue, tx_ring_ref, rx_ring_ref,
 tx_evtchn, rx_evtchn);
-   if (err) {
+   if (err)
xenbus_dev_fatal(dev, err,
 "mapping shared-frames %lu/%lu port tx %u rx 
%u",
 tx_ring_ref, rx_ring_ref,
 tx_evtchn, rx_evtchn);
-   goto err;
-   }
 
 err: /* Regular return falls through with err == 0 */
kfree(xspath);
-- 
2.6.3

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

Re: [PATCH] stmmac: Don't exit mdio registration when mdio subnode is not found in the DTS

[Romain Perier]

Hi all,

2016-01-02 6:51 GMT+01:00 Florian Fainelli :
> On December 29, 2015 6:05:35 AM PST, Romain Perier  
> wrote:
>>Originally, most of the platforms using this driver did not define an
>>mdio subnode
>>in the devicetree. Commit e34d65 ("stmmac: create of compatible mdio
>>bus for stmmac driver")
>>introduced a backward compatibily issue by using of_mdiobus_register
>>explicitly
>>with an mdio subnode. This patch fixes the issue by calling the
>>function
>>mdiobus_register, when mdio subnode is not found. The driver is now
>>compatible
>>with both modes.
>
> Looks reasonable to me, though you will want to make sure the different DTSes 
> get updated to include the proper compatible node for the MDIO bus.
>
>>
>>Signed-off-by: Romain Perier 
>
> Please include a Fixes tag to help keep track of changes.

Sure, sorry for my ignorance, but I did not know that we're supposed
to add a Fixes tag in this situation.

>
>>---
>> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 8 +---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
>>b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
>>index 16c85cc..0034de44 100644
>>--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
>>+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
>>@@ -218,8 +218,7 @@ int stmmac_mdio_register(struct net_device *ndev)
>>   if (mdio_node) {
>>   netdev_dbg(ndev, "FOUND MDIO subnode\n");
>>   } else {
>>-  netdev_err(ndev, "NO MDIO subnode\n");
>>-  return 0;
>>+  netdev_warn(ndev, "No MDIO subnode found\n");
>>   }
>>   }
>>
>>@@ -251,7 +250,10 @@ int stmmac_mdio_register(struct net_device *ndev)
>>   new_bus->phy_mask = mdio_bus_data->phy_mask;
>>   new_bus->parent = priv->device;
>>
>>-  err = of_mdiobus_register(new_bus, mdio_node);
>>+  if (IS_ENABLED(CONFIG_OF) && mdio_node)
>

> You should be able to drop the IS_ENABLED part since there is an inline 
> provided in the non-OF case which does a fallback to mdiobus_register().

Good point

>
>>+  err = of_mdiobus_register(new_bus, mdio_node);
>>+  else
>>+  err = mdiobus_register(new_bus);
>>   if (err != 0) {
>
> This looks reasonable, does it work if we just assign mdio_node to the 
> pdev->dev.of_node to be backwards compatible with DTSes which have not yet 
> been updated?
>

I don't understand what you mean. Instead of defining mdio_node with
NULL, you propose to assign it to pdev->dev.of_node as its default
value... then it would be reassigned with "child_nod" if a subnode
with the compatible string "snps,dwmac-mdio" is found, or kept with
its default value otherwise... and in this case we could call
of_mdiobus_register(new_bus, mdio_node) all the time ? (it removes the
conditonnal branch)

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

net-rsi: Reconsider usage of variable "vap_id" in rsi_send_mgmt_pkt()

[SF Markus Elfring]

Hello,

I have taken another look at the implementation of the function 
"rsi_send_mgmt_pkt".
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/net/wireless/rsi/rsi_91x_pkt.c?id=e8c58e7a5a106c3d557fccd01cd4d1128f9bab38#n114

I find the following statement combination interesting there.

…
u8 vap_id = 0;
…
msg[7] |= cpu_to_le16(vap_id << 8);
…

I would appreciate a further clarification.
Does a shift operation for a variable which contains zero indicate an open 
issue?

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

[PATCH 3/5] xen-netback: Replace a variable initialisation by an assignment in read_xenbus_vif_flags()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 18:01:57 +0100

Replace an explicit initialisation for one local variable at the beginning
by an assignment.

Signed-off-by: Markus Elfring 
---
 drivers/net/xen-netback/xenbus.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index d4947e1..aff963f 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -946,7 +946,7 @@ err: /* Regular return falls through with err == 0 */
 
 static int read_xenbus_vif_flags(struct backend_info *be)
 {
-   struct xenvif *vif = be->vif;
+   struct xenvif *vif;
struct xenbus_device *dev = be->dev;
unsigned int rx_copy;
int err, val;
@@ -968,13 +968,14 @@ static int read_xenbus_vif_flags(struct backend_info *be)
if (xenbus_scanf(XBT_NIL, dev->otherend,
 "feature-rx-notify", "%d", ) < 0)
val = 0;
+   vif = be->vif;
if (!val) {
/* - Reduce drain timeout to poll more frequently for
 *   Rx requests.
 * - Disable Rx stall detection.
 */
-   be->vif->drain_timeout = msecs_to_jiffies(30);
-   be->vif->stall_timeout = 0;
+   vif->drain_timeout = msecs_to_jiffies(30);
+   vif->stall_timeout = 0;
}
 
if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
-- 
2.6.3

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

[PATCH v2 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 19:22:36 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c 
b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..571eaba 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
  struct sk_buff *skb)
 {
struct rsi_hw *adapter = common->priv;
-   struct ieee80211_hdr *wh = NULL;
+   struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
-   struct ieee80211_bss_conf *bss = NULL;
+   struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = >conf;
struct skb_info *tx_params;
int status = -E2BIG;
-   __le16 *msg = NULL;
-   u8 extnd_size = 0;
+   __le16 *msg;
+   u8 extnd_size;
u8 vap_id = 0;
 
info = IEEE80211_SKB_CB(skb);
-- 
2.6.3

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

Re: [PATCH] stmmac: Don't exit mdio registration when mdio subnode is not found in the DTS

[Florian Fainelli]

Le 02/01/2016 07:11, Romain Perier a écrit :
>>> Signed-off-by: Romain Perier 
>>
>> Please include a Fixes tag to help keep track of changes.
> 
> Sure, sorry for my ignorance, but I did not know that we're supposed
> to add a Fixes tag in this situation.

It is a good practice and definitively helps making sure that if
regression entered a merge window, and a fix came in another merge
window, people can easily backport the fix, moving on..

[snip]

>> This looks reasonable, does it work if we just assign mdio_node to the 
>> pdev->dev.of_node to be backwards compatible with DTSes which have not yet 
>> been updated?
>>
> 
> I don't understand what you mean. Instead of defining mdio_node with
> NULL, you propose to assign it to pdev->dev.of_node as its default
> value... then it would be reassigned with "child_nod" if a subnode
> with the compatible string "snps,dwmac-mdio" is found, or kept with
> its default value otherwise... and in this case we could call
> of_mdiobus_register(new_bus, mdio_node) all the time ? (it removes the
> conditonnal branch)

Your understanding is correct, this is what I was suggesting, thanks
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] xen-netback: Fine-tuning for three function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 18:46:45 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Delete an unnecessary assignment in connect_rings()
  Delete an unnecessary goto statement in connect_rings()
  Replace a variable initialisation by an assignment in read_xenbus_vif_flags()
  Replace a variable initialisation by an assignment in xen_register_watchers()
  Delete an unnecessary variable initialisation in xen_register_watchers()

 drivers/net/xen-netback/xenbus.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
2.6.3

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

Re: [PATCH 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

[Francois Romieu]

SF Markus Elfring  :
> From: Markus Elfring 
> Date: Sat, 2 Jan 2016 15:25:34 +0100
> 
> Replace explicit initialisation for two local variables at the beginning
> by assignments.

It makes no sense for the 'adapter' variable.

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

Disable "received packet with own address as source" check

[Luca Dionisi]

My linux box has a duty. It has to forward IP packets for a "private"
network which it doesn't belong to.

The network is "private" has the meaning that the hosts that belong to
the network have IP addresses that are unique only between them.

Say that the network is formed by 4 nodes: "a", "b", "c", "d", with
their IP addresses: 10.0.1.1, ..., 10.0.1.4.

The nodes "a", "b" and "c" are connected, but the only way from "c" to
"d" is through my box, let's call it "x".

The problem is that "x" has to live inside another "private" network,
and in this other network the address of "x" is 10.0.1.4.

When I send a PING-request from "c" to "d" the packet goes to a
specific NIC of "x". Thanks to some carefully designed
iptables+ip-rule tricks in "x", the PING-request is effectively
relayed from "x" to the node "d", although "x" has the same IP as the
destination of the packet.

Now the node "d" sends a PING-reply to "c". This packet goes to a
specific NIC of "x". There, the same tricks would send the packet to
"c". But this is not happening.

I suspect that the problem is that the box "x" refuses to relay a
packet that has an IP source equal to one of its local IP addresses,
although this IP is assigned to another NIC.

I have this suspect because of the tcpdump traces that I have
collected. But I don't seem to succeed in finding other evidences (I
looked into dmesg and /var/log/syslog).

Is there a tunable in linux to change this behaviour? I want to say to
"x": When a packet comes in through NIC0, relay it without bothering
if it has your IP as source address.

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

[PATCH 4/5] xen-netback: Replace a variable initialisation by an assignment in xen_register_watchers()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 18:23:16 +0100

Replace an explicit initialisation for one local variable at the beginning
by an assignment.

Signed-off-by: Markus Elfring 
---
 drivers/net/xen-netback/xenbus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index aff963f..e8dfc3d 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -687,11 +687,12 @@ static int xen_register_watchers(struct xenbus_device 
*dev, struct xenvif *vif)
 {
int err = 0;
char *node;
-   unsigned maxlen = strlen(dev->nodename) + sizeof("/rate");
+   unsigned maxlen;
 
if (vif->credit_watch.node)
return -EADDRINUSE;
 
+   maxlen = strlen(dev->nodename) + sizeof("/rate");
node = kmalloc(maxlen, GFP_KERNEL);
if (!node)
return -ENOMEM;
-- 
2.6.3

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

Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

[kbuild test robot]

Hi Markus,

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on v4.4-rc7 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/net-rsi-Fine-tuning-for-two-function-implementations/20160102-224740
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git 
master
config: x86_64-randconfig-s2-01030012 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/wireless/rsi/rsi_91x_pkt.c: In function 'rsi_send_mgmt_pkt':
>> drivers/net/wireless/rsi/rsi_91x_pkt.c:211:2: warning: 'status' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
 rsi_indicate_tx_status(common->priv, skb, status);
 ^

vim +/status +211 drivers/net/wireless/rsi/rsi_91x_pkt.c

dad0d04f Fariya Fatima 2014-03-16  195  /* Indicate to firmware to give 
cfm */
dad0d04f Fariya Fatima 2014-03-16  196  if ((skb->data[16] == 
IEEE80211_STYPE_PROBE_REQ) && (!bss->assoc)) {
dad0d04f Fariya Fatima 2014-03-16  197  msg[1] |= 
cpu_to_le16(BIT(10));
dad0d04f Fariya Fatima 2014-03-16  198  msg[7] = 
cpu_to_le16(PROBEREQ_CONFIRM);
dad0d04f Fariya Fatima 2014-03-16  199  common->mgmt_q_block = 
true;
dad0d04f Fariya Fatima 2014-03-16  200  }
dad0d04f Fariya Fatima 2014-03-16  201  
dad0d04f Fariya Fatima 2014-03-16  202  msg[7] |= cpu_to_le16(vap_id << 
8);
dad0d04f Fariya Fatima 2014-03-16  203  
dad0d04f Fariya Fatima 2014-03-16  204  status = 
adapter->host_intf_write_pkt(common->priv,
dad0d04f Fariya Fatima 2014-03-16  205  
  (u8 *)msg,
dad0d04f Fariya Fatima 2014-03-16  206  
  skb->len);
dad0d04f Fariya Fatima 2014-03-16  207  if (status)
dad0d04f Fariya Fatima 2014-03-16  208  rsi_dbg(ERR_ZONE, "%s: 
Failed to write the packet\n", __func__);
dad0d04f Fariya Fatima 2014-03-16  209  
dad0d04f Fariya Fatima 2014-03-16  210  err:
dad0d04f Fariya Fatima 2014-03-16 @211  
rsi_indicate_tx_status(common->priv, skb, status);
dad0d04f Fariya Fatima 2014-03-16  212  return status;
dad0d04f Fariya Fatima 2014-03-16  213  }

:: The code at line 211 was first introduced by commit
:: dad0d04fa7ba41ce603a01e8e64967650303e9a2 rsi: Add RS9113 wireless driver

:: TO: Fariya Fatima <fari...@gmail.com>
:: CC: John W. Linville <linvi...@tuxdriver.com>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: Binary data

[PATCH 5/5] xen-netback: Delete an unnecessary variable initialisation in xen_register_watchers()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 2 Jan 2016 18:28:26 +0100

Omit explicit initialisation at the beginning for one local variable
that is redefined before its first use.

Signed-off-by: Markus Elfring 
---
 drivers/net/xen-netback/xenbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index e8dfc3d..55f0735 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -685,7 +685,7 @@ static void xen_net_rate_changed(struct xenbus_watch *watch,
 
 static int xen_register_watchers(struct xenbus_device *dev, struct xenvif *vif)
 {
-   int err = 0;
+   int err;
char *node;
unsigned maxlen;
 
-- 
2.6.3

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

Re: [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init()

[Souptick Joarder]

On Sat, Jan 2, 2016 at 2:02 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Fri, 1 Jan 2016 21:16:01 +0100
>
> Rename a jump label according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/net/wireless/intel/iwlegacy/common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/common.c 
> b/drivers/net/wireless/intel/iwlegacy/common.c
> index ae45fd3..660ab2b 100644
> --- a/drivers/net/wireless/intel/iwlegacy/common.c
> +++ b/drivers/net/wireless/intel/iwlegacy/common.c
> @@ -759,7 +759,7 @@ il_eeprom_init(struct il_priv *il)
>  IL_EEPROM_ACCESS_TIMEOUT);
> if (ret < 0) {
> IL_ERR("Time out reading EEPROM[%d]\n", addr);
> -   goto done;
> +   goto release_semaphore;

Current code looks good.
> }
> r = _il_rd(il, CSR_EEPROM_REG);
> e[addr / 2] = cpu_to_le16(r >> 16);
> @@ -769,7 +769,7 @@ il_eeprom_init(struct il_priv *il)
>  il_eeprom_query16(il, EEPROM_VERSION));
>
> ret = 0;
> -done:
> +release_semaphore:
> il->ops->eeprom_release_semaphore(il);
>
> if (ret) {
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 1/2] net-qmi_wwan: Refactoring for qmi_wwan_bind()

[Bjørn Mork]

SF Markus Elfring  writes:

> From: Markus Elfring 
> Date: Fri, 1 Jan 2016 17:32:07 +0100
>
> Reduce the scope for the local variable "desc" to one branch
> of an if statement.

This patch is harmless.  But is also pointless.

You could at least try to explain why this must be changed.  I'm not
interested in why you think it is better this way - I might agree with
that.  what I am interested in is the advantage changing the code gives
us.  Some analysis of the risk and work involved would also be nice.  Is
this change really worth it?

Personally I am convinced that I wasted any time I used writing this,
and you wasted any time you used reading it.  Sorry.

Note:  This patch would have been fine if it was a natural part of some
*improvement* of the driver, i.e. a bugfix or feaure addition. As a
standalone patch I see it as noise.

Please stop the noise and start writing something useful.  I'm sure you
can fix bugs instead.  Wouldn't that be more interesting?  More
challenging?  These mindless robotic code refactoring patches are really
best left for robots.




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

Re: [PATCH next] net/core/dev: Warn on an impossibly short offload frame

[Joe Perches]

On Sat, 2016-01-02 at 19:25 -0500, Aaron Conole wrote:
> When signaling that a GRO frame is ready to be processed, the network stack
> correctly checks length and aborts processing when a frame is less than 14
> bytes. However, such a condition is really indicative of a broken driver,
> and should be loudly signaled, rather than silently dropped as the case is
> today.
> 
> Convert the condition to use WARN_ON() to ensure that the stack loudly
> complains about such broken drivers.
[]
> diff --git a/net/core/dev.c b/net/core/dev.c
[]
> @@ -4579,7 +4579,7 @@ static struct sk_buff *napi_frags_skb(struct 
> napi_struct *napi)
>   eth = skb_gro_header_fast(skb, 0);
>   if (unlikely(skb_gro_header_hard(skb, hlen))) {
>   eth = skb_gro_header_slow(skb, hlen, 0);
> - if (unlikely(!eth)) {
> + if (WARN_ON(!eth)) {
>   napi_reuse_skb(napi, skb);
>   return NULL;
>   }

It's generally a good idea to use
WARN_ON_RATELIMIT or WARN_ON_ONCE.

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

Re: [PATCH v2 net-next 3/4] soreuseport: setsockopt SO_ATTACH_REUSEPORT_[CE]BPF

[Daniel Borkmann]

On 12/29/2015 06:29 PM, Craig Gallek wrote:

From: Craig Gallek 

Expose socket options for setting a classic or extended BPF program
for use when selecting sockets in an SO_REUSEPORT group.  These options
can be used on the first socket to belong to a group before bind or
on any socket in the group after bind.

This change includes refactoring of the existing sk_filter code to
allow reuse of the existing BPF filter validation checks.

Signed-off-by: Craig Gallek 

[...]

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 4165e9a..3561d3a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -447,6 +447,8 @@ void bpf_prog_destroy(struct bpf_prog *fp);

  int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
  int sk_attach_bpf(u32 ufd, struct sock *sk);
+int reuseport_attach_filter(struct sock_fprog *fprog, struct sock *sk);
+int reuseport_attach_bpf(u32 ufd, struct sock *sk);


Maybe for consistency this should be sk_* prefixed as well due to its
relation to sockets?


  int sk_detach_filter(struct sock *sk);
  int sk_get_filter(struct sock *sk, struct sock_filter __user *filter,
  unsigned int len);

[...]

diff --git a/net/core/filter.c b/net/core/filter.c
index c770196..81eeeae 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -50,6 +50,7 @@
  #include 
  #include 
  #include 
+#include 

  /**
   *sk_filter - run a packet through a socket filter
@@ -1167,17 +1168,32 @@ static int __sk_attach_prog(struct bpf_prog *prog, 
struct sock *sk)
return 0;
  }

-/**
- * sk_attach_filter - attach a socket filter
- * @fprog: the filter program
- * @sk: the socket to use
- *
- * Attach the user's filter code. We first run some sanity checks on
- * it to make sure it does not explode on us later. If an error
- * occurs or there is insufficient memory for the filter a negative
- * errno code is returned. On success the return is zero.
- */
-int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
+static int __reuseport_attach_prog(struct bpf_prog *prog, struct sock *sk)
+{
+   struct bpf_prog *old_prog;
+   int err;
+
+   if (bpf_prog_size(prog->len) > sysctl_optmem_max)
+   return -ENOMEM;


You currently don't charge the BPF program against the optmem limits, but just
test if the size of a given program would surpass the current sysctl_optmem_max.
Ok, after all, this would block anything beyond 2560 insns by default. Is there
a reason it's not charged for real? Due to the sysctl_optmem_max default being
too small?

Btw, in case of an eBPF fd, we already charged it to the user's RLIMIT_MEMLOCK,
not sure if blocking it here after program already got an fd makes much sense.
I'm fine if you want to leave it for now and refine this later, though.


+   if (sk_unhashed(sk)) {
+   err = reuseport_alloc(sk);
+   if (err)
+   return err;
+   } else if (!rcu_access_pointer(sk->sk_reuseport_cb)) {
+   /* The socket wasn't bound with SO_REUSEPORT */
+   return -EINVAL;
+   }
+
+   old_prog = reuseport_attach_prog(sk, prog);
+   if (old_prog)
+   bpf_prog_destroy(old_prog);
+
+   return 0;
+}
+
+static
+struct bpf_prog *__get_filter(struct sock_fprog *fprog, struct sock *sk)
  {
unsigned int fsize = bpf_classic_proglen(fprog);
unsigned int bpf_fsize = bpf_prog_size(fprog->len);
@@ -1185,19 +1201,19 @@ int sk_attach_filter(struct sock_fprog *fprog, struct 
sock *sk)
int err;

if (sock_flag(sk, SOCK_FILTER_LOCKED))
-   return -EPERM;
+   return ERR_PTR(-EPERM);

/* Make sure new filter is there and in the right amounts. */
if (fprog->filter == NULL)
-   return -EINVAL;
+   return ERR_PTR(-EINVAL);

prog = bpf_prog_alloc(bpf_fsize, 0);
if (!prog)
-   return -ENOMEM;
+   return ERR_PTR(-ENOMEM);

if (copy_from_user(prog->insns, fprog->filter, fsize)) {
__bpf_prog_free(prog);
-   return -EFAULT;
+   return ERR_PTR(-EFAULT);
}

prog->len = fprog->len;
@@ -1205,13 +1221,31 @@ int sk_attach_filter(struct sock_fprog *fprog, struct 
sock *sk)
err = bpf_prog_store_orig_filter(prog, fprog);
if (err) {
__bpf_prog_free(prog);
-   return -ENOMEM;
+   return ERR_PTR(-ENOMEM);
}

/* bpf_prepare_filter() already takes care of freeing
 * memory in case something goes wrong.
 */
prog = bpf_prepare_filter(prog, NULL);
+   return prog;
+}


Nit: return bpf_prepare_filter(prog, NULL);

Rest of BPF bits look good to me.

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH 2/2] net-qmi_wwan: Delete an unnecessary variable initialisation in qmi_wwan_register_subdriver()

[David Miller]

From: Bjørn Mork 
Date: Sat, 02 Jan 2016 22:30:48 +0100

> SF Markus Elfring  writes:
> 
>> From: Markus Elfring 
>> Date: Fri, 1 Jan 2016 17:35:03 +0100
>>
>> Omit explicit initialisation at the beginning for one local variable
>> that is redefined before its first use.
> 
> 
> This patch is unnecessary. The variable initialisation is redundant.
> See the difference?  Sending an unnecessary patch causes unnecessary
> load on reviewers and maintainers.  Keeping redundant code has no
> measurable cost, and can save the same maintainers a lot of trouble
> later.

+1

I'm getting really tired of these changes.  And a lot of them are
coming in right now...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] net-qmi_wwan: Delete an unnecessary variable initialisation in qmi_wwan_register_subdriver()

[Bjørn Mork]

SF Markus Elfring  writes:

> From: Markus Elfring 
> Date: Fri, 1 Jan 2016 17:35:03 +0100
>
> Omit explicit initialisation at the beginning for one local variable
> that is redefined before its first use.


This patch is unnecessary. The variable initialisation is redundant.
See the difference?  Sending an unnecessary patch causes unnecessary
load on reviewers and maintainers.  Keeping redundant code has no
measurable cost, and can save the same maintainers a lot of trouble
later.

I'd like to keep this particular redundant initialisation as a safe
guard against future code refactoring, causing for example the err label
to move up.  Yes, I do understand that any patch with such a bug should
be rejected, but I do know what happens in the real world and how easy
it is for something like that to slip through in a stream of unnecessary
"cleanup" patches.

Reducing redundancy in the kernel is only making the code less robust.
It is harmful. Please stop.  Thanks.


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

Charity Donation

[Jeff Skoll]

Hi,
My name is Jeffrey Skoll, a philanthropist and the founder of one of the 
largest private foundations in the world. I believe strongly in ‘giving while 
living.’ I had one idea that never changed in my mind — that you should use 
your wealth to help people and I have decided to secretly give USD2.498 Million 
to a randomly selected individual. On receipt of this email, you should count 
yourself as the individual. Kindly get back to me at your earliest convenience, 
so I know your email address is valid.

Visit the web page to know more about me: 
http://www.theglobeandmail.com/news/national/meet-the-canadian-billionaire-whos-giving-it-all-away/article4209888/
 or you can read an article of me on Wikipedia.

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

[PATCH next] net/core/dev: Warn on an impossibly short offload frame

[Aaron Conole]

When signaling that a GRO frame is ready to be processed, the network stack
correctly checks length and aborts processing when a frame is less than 14
bytes. However, such a condition is really indicative of a broken driver,
and should be loudly signaled, rather than silently dropped as the case is
today.

Convert the condition to use WARN_ON() to ensure that the stack loudly
complains about such broken drivers.

Signed-off-by: Aaron Conole 
---
 net/core/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 914b4a2..8af4e29 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4579,7 +4579,7 @@ static struct sk_buff *napi_frags_skb(struct napi_struct 
*napi)
eth = skb_gro_header_fast(skb, 0);
if (unlikely(skb_gro_header_hard(skb, hlen))) {
eth = skb_gro_header_slow(skb, hlen, 0);
-   if (unlikely(!eth)) {
+   if (WARN_ON(!eth)) {
napi_reuse_skb(napi, skb);
return NULL;
}
-- 
2.6.4

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

Re: [PATCH 0/5] xen-netback: Fine-tuning for three function implementations

[Joe Perches]

On Sat, 2016-01-02 at 18:50 +0100, SF Markus Elfring wrote:
> A few update suggestions were taken into account
> from static source code analysis.

While static analysis can be useful, I don't think these
specific conversions are generally useful.

Perhaps it would be more useful to convert the string
duplication or snprintf logic to kstrdup/kasprintf

This:

if (num_queues == 1) {
xspath = kzalloc(strlen(dev->otherend) + 1, GFP_KERNEL);
if (!xspath) {
xenbus_dev_fatal(dev, -ENOMEM,
 "reading ring references");
return -ENOMEM;
}
strcpy(xspath, dev->otherend);
} else {
xspathsize = strlen(dev->otherend) + xenstore_path_ext_size;
xspath = kzalloc(xspathsize, GFP_KERNEL);
if (!xspath) {
xenbus_dev_fatal(dev, -ENOMEM,
 "reading ring references");
return -ENOMEM;
}
snprintf(xspath, xspathsize, "%s/queue-%u", dev->otherend,
 queue->id);
}

could be simplified to something like:

if (num_queues == 1)
xspath = kstrdup(dev->otherend, GFP_KERNEL);
else
xspath = kasprintf(GFP_KERNEL, "%s/queue-%u",
   dev->otherend, queue->id);
if (!xspath)
etc...

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

[PATCH] igb: constify e1000_phy_operations structure

[Julia Lawall]

This e1000_phy_operations structure is never modified, so declare it as
const.  Other structures of this type are already const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/net/ethernet/intel/igb/e1000_82575.c |2 +-
 drivers/net/ethernet/intel/igb/e1000_hw.h|2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c 
b/drivers/net/ethernet/intel/igb/e1000_82575.c
index adb33e2..1ea27bf 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -2889,7 +2889,7 @@ static struct e1000_mac_operations e1000_mac_ops_82575 = {
 #endif
 };
 
-static struct e1000_phy_operations e1000_phy_ops_82575 = {
+static const struct e1000_phy_operations e1000_phy_ops_82575 = {
.acquire  = igb_acquire_phy_82575,
.get_cfg_done = igb_get_cfg_done_82575,
.release  = igb_release_phy_82575,
diff --git a/drivers/net/ethernet/intel/igb/e1000_hw.h 
b/drivers/net/ethernet/intel/igb/e1000_hw.h
index 4034207..a34d640 100644
--- a/drivers/net/ethernet/intel/igb/e1000_hw.h
+++ b/drivers/net/ethernet/intel/igb/e1000_hw.h
@@ -372,7 +372,7 @@ struct e1000_thermal_sensor_data {
 struct e1000_info {
s32 (*get_invariants)(struct e1000_hw *);
struct e1000_mac_operations *mac_ops;
-   struct e1000_phy_operations *phy_ops;
+   const struct e1000_phy_operations *phy_ops;
struct e1000_nvm_operations *nvm_ops;
 };
 

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

[PATCH] ixgbe: constify ixgbe_mbx_operations structure

[Julia Lawall]

This ixgbe_mbx_operations structure is never modified, so declare it as
const.  The other structure of this type is already const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c  |2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h  |2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c
index 9993a47..4a769db 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c
@@ -446,7 +446,7 @@ void ixgbe_init_mbx_params_pf(struct ixgbe_hw *hw)
 }
 #endif /* CONFIG_PCI_IOV */
 
-struct ixgbe_mbx_operations mbx_ops_generic = {
+const struct ixgbe_mbx_operations mbx_ops_generic = {
.read   = ixgbe_read_mbx_pf,
.write  = ixgbe_write_mbx_pf,
.read_posted= ixgbe_read_posted_mbx,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index 8daa95f..9119cb8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -123,6 +123,6 @@ s32 ixgbe_check_for_rst(struct ixgbe_hw *, u16);
 void ixgbe_init_mbx_params_pf(struct ixgbe_hw *);
 #endif /* CONFIG_PCI_IOV */
 
-extern struct ixgbe_mbx_operations mbx_ops_generic;
+extern const struct ixgbe_mbx_operations mbx_ops_generic;
 
 #endif /* _IXGBE_MBX_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 5f53cc6..0edc2c2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3477,7 +3477,7 @@ struct ixgbe_info {
struct ixgbe_mac_operations *mac_ops;
struct ixgbe_eeprom_operations  *eeprom_ops;
struct ixgbe_phy_operations *phy_ops;
-   struct ixgbe_mbx_operations *mbx_ops;
+   const struct ixgbe_mbx_operations *mbx_ops;
const u32   *mvals;
 };
 

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

[PATCH net-next 3/7] bnxt_en: Check for NULL rx or tx ring.

[Michael Chan]

Each bnxt_napi structure may no longer be having both an rx ring and
a tx ring.  Check for a valid ring before using it.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8635e03..11478c9 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1867,6 +1867,9 @@ static void bnxt_init_ring_struct(struct bnxt *bp)
ring->vmem_size = 0;
 
rxr = bnapi->rx_ring;
+   if (!rxr)
+   goto skip_rx;
+
ring = >rx_ring_struct;
ring->nr_pages = bp->rx_nr_pages;
ring->page_size = HW_RXBD_RING_SIZE;
@@ -1883,7 +1886,11 @@ static void bnxt_init_ring_struct(struct bnxt *bp)
ring->vmem_size = SW_RXBD_AGG_RING_SIZE * bp->rx_agg_nr_pages;
ring->vmem = (void **)>rx_agg_ring;
 
+skip_rx:
txr = bnapi->tx_ring;
+   if (!txr)
+   continue;
+
ring = >tx_ring_struct;
ring->nr_pages = bp->tx_nr_pages;
ring->page_size = HW_RXBD_RING_SIZE;
@@ -2400,13 +2407,17 @@ static void bnxt_clear_ring_indices(struct bnxt *bp)
cpr->cp_raw_cons = 0;
 
txr = bnapi->tx_ring;
-   txr->tx_prod = 0;
-   txr->tx_cons = 0;
+   if (txr) {
+   txr->tx_prod = 0;
+   txr->tx_cons = 0;
+   }
 
rxr = bnapi->rx_ring;
-   rxr->rx_prod = 0;
-   rxr->rx_agg_prod = 0;
-   rxr->rx_sw_agg_prod = 0;
+   if (rxr) {
+   rxr->rx_prod = 0;
+   rxr->rx_agg_prod = 0;
+   rxr->rx_sw_agg_prod = 0;
+   }
}
 }
 
@@ -4999,6 +5010,9 @@ static void bnxt_dump_tx_sw_state(struct bnxt_napi *bnapi)
struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
int i = bnapi->index;
 
+   if (!txr)
+   return;
+
netdev_info(bnapi->bp->dev, "[%d]: tx{fw_ring: %d prod: %x cons: %x}\n",
i, txr->tx_ring_struct.fw_ring_id, txr->tx_prod,
txr->tx_cons);
@@ -5009,6 +5023,9 @@ static void bnxt_dump_rx_sw_state(struct bnxt_napi *bnapi)
struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
int i = bnapi->index;
 
+   if (!rxr)
+   return;
+
netdev_info(bnapi->bp->dev, "[%d]: rx{fw_ring: %d prod: %x} 
rx_agg{fw_ring: %d agg_prod: %x sw_agg_prod: %x}\n",
i, rxr->rx_ring_struct.fw_ring_id, rxr->rx_prod,
rxr->rx_agg_ring_struct.fw_ring_id, rxr->rx_agg_prod,
-- 
1.8.3.1

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

[PATCH net-next 4/7] bnxt_en: Re-structure ring indexing and mapping.

[Michael Chan]

In order to support dedicated or shared completion rings, the ring
indexing and mapping are re-structured as below:

1. bp->grp_info[] array index is 1:1 with bp->bnapi[] array index and
completion ring index.

2. rx rings 0 to n will be mapped to completion rings 0 to n.

3. If tx and rx rings share completion rings, then tx rings 0 to m will
be mapped to completion rings 0 to m.

4. If tx and rx rings use dedicated completion rings, then tx rings 0 to
m will be mapped to completion rings n + 1 to n + m.

5. Each tx or rx ring will use the corresponding completion ring index
for doorbell mapping and MSIX mapping.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 91 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  2 +
 2 files changed, 55 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 11478c9..8a54dab 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -422,7 +422,7 @@ tx_dma_error:
 static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
 {
struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
-   int index = bnapi->index;
+   int index = txr - >tx_ring[0];
struct netdev_queue *txq = netdev_get_tx_queue(bp->dev, index);
u16 cons = txr->tx_cons;
struct pci_dev *pdev = bp->pdev;
@@ -3082,7 +3082,7 @@ static int bnxt_hwrm_vnic_ctx_alloc(struct bnxt *bp, u16 
vnic_id)
 
 static int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 vnic_id)
 {
-   int grp_idx = 0;
+   unsigned int ring = 0, grp_idx;
struct bnxt_vnic_info *vnic = >vnic_info[vnic_id];
struct hwrm_vnic_cfg_input req = {0};
 
@@ -3093,10 +3093,11 @@ static int bnxt_hwrm_vnic_cfg(struct bnxt *bp, u16 
vnic_id)
req.rss_rule = cpu_to_le16(vnic->fw_rss_cos_lb_ctx);
req.cos_rule = cpu_to_le16(0x);
if (vnic->flags & BNXT_VNIC_RSS_FLAG)
-   grp_idx = 0;
+   ring = 0;
else if (vnic->flags & BNXT_VNIC_RFS_FLAG)
-   grp_idx = vnic_id - 1;
+   ring = vnic_id - 1;
 
+   grp_idx = bp->rx_ring[ring].bnapi->index;
req.vnic_id = cpu_to_le16(vnic->fw_vnic_id);
req.dflt_ring_grp = cpu_to_le16(bp->grp_info[grp_idx].fw_grp_id);
 
@@ -3137,22 +3138,25 @@ static void bnxt_hwrm_vnic_free(struct bnxt *bp)
bnxt_hwrm_vnic_free_one(bp, i);
 }
 
-static int bnxt_hwrm_vnic_alloc(struct bnxt *bp, u16 vnic_id, u16 start_grp_id,
-   u16 end_grp_id)
+static int bnxt_hwrm_vnic_alloc(struct bnxt *bp, u16 vnic_id,
+   unsigned int start_rx_ring_idx,
+   unsigned int nr_rings)
 {
-   u32 rc = 0, i, j;
+   int rc = 0;
+   unsigned int i, j, grp_idx, end_idx = start_rx_ring_idx + nr_rings;
struct hwrm_vnic_alloc_input req = {0};
struct hwrm_vnic_alloc_output *resp = bp->hwrm_cmd_resp_addr;
 
/* map ring groups to this vnic */
-   for (i = start_grp_id, j = 0; i < end_grp_id; i++, j++) {
-   if (bp->grp_info[i].fw_grp_id == INVALID_HW_RING_ID) {
+   for (i = start_rx_ring_idx, j = 0; i < end_idx; i++, j++) {
+   grp_idx = bp->rx_ring[i].bnapi->index;
+   if (bp->grp_info[grp_idx].fw_grp_id == INVALID_HW_RING_ID) {
netdev_err(bp->dev, "Not enough ring groups avail:%x 
req:%x\n",
-  j, (end_grp_id - start_grp_id));
+  j, nr_rings);
break;
}
bp->vnic_info[vnic_id].fw_grp_ids[j] =
-   bp->grp_info[i].fw_grp_id;
+   bp->grp_info[grp_idx].fw_grp_id;
}
 
bp->vnic_info[vnic_id].fw_rss_cos_lb_ctx = INVALID_HW_RING_ID;
@@ -3179,20 +3183,22 @@ static int bnxt_hwrm_ring_grp_alloc(struct bnxt *bp)
struct hwrm_ring_grp_alloc_input req = {0};
struct hwrm_ring_grp_alloc_output *resp =
bp->hwrm_cmd_resp_addr;
+   unsigned int grp_idx = bp->rx_ring[i].bnapi->index;
 
bnxt_hwrm_cmd_hdr_init(bp, , HWRM_RING_GRP_ALLOC, -1, -1);
 
-   req.cr = cpu_to_le16(bp->grp_info[i].cp_fw_ring_id);
-   req.rr = cpu_to_le16(bp->grp_info[i].rx_fw_ring_id);
-   req.ar = cpu_to_le16(bp->grp_info[i].agg_fw_ring_id);
-   req.sc = cpu_to_le16(bp->grp_info[i].fw_stats_ctx);
+   req.cr = cpu_to_le16(bp->grp_info[grp_idx].cp_fw_ring_id);
+   req.rr = cpu_to_le16(bp->grp_info[grp_idx].rx_fw_ring_id);
+   req.ar = cpu_to_le16(bp->grp_info[grp_idx].agg_fw_ring_id);
+   req.sc = cpu_to_le16(bp->grp_info[grp_idx].fw_stats_ctx);
 

[PATCH net-next 2/7] bnxt_en: Separate bnxt_{rx|tx}_ring_info structs from bnxt_napi struct.

[Michael Chan]

Currently, an rx and a tx ring are always paired with a completion ring.
We want to restructure it so that it is possible to have a dedicated
completion ring for tx or rx only.

The bnxt hardware uses a completion ring for rx and tx events.  The driver
has to process the completion ring entries sequentially for the rx and tx
events.  Using a dedicated completion ring for rx only or tx only has these
benefits:

1. A burst of rx packets can cause delay in processing tx events if the
completion ring is shared.  If tx queue is stopped by BQL, this can cause
delay in re-starting the tx queue.

2. A completion ring is sized according to the rx and tx ring size rounded
up to the nearest power of 2.  When the completion ring is shared, it is
sized by adding the rx and tx ring sizes and then rounded to the next power
of 2, often with a lot of wasted space.

3. Using dedicated completion ring, we can adjust the tx and rx coalescing
parameters independently for rx and tx.

The first step is to separate the rx and tx ring structures from the
bnxt_napi struct.

In this patch, an rx ring and a tx ring will point to the same bnxt_napi
struct to share the same completion ring.  No change in ring assignment
and mapping yet.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 147 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |   9 +-
 2 files changed, 70 insertions(+), 86 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 3f053de..8635e03 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -179,7 +179,6 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, 
struct net_device *dev)
u32 len, free_size, vlan_tag_flags, cfa_action, flags;
u16 prod, last_frag;
struct pci_dev *pdev = bp->pdev;
-   struct bnxt_napi *bnapi;
struct bnxt_tx_ring_info *txr;
struct bnxt_sw_tx_bd *tx_buf;
 
@@ -189,8 +188,7 @@ static netdev_tx_t bnxt_start_xmit(struct sk_buff *skb, 
struct net_device *dev)
return NETDEV_TX_OK;
}
 
-   bnapi = bp->bnapi[i];
-   txr = >tx_ring;
+   txr = >tx_ring[i];
txq = netdev_get_tx_queue(dev, i);
prod = txr->tx_prod;
 
@@ -423,7 +421,7 @@ tx_dma_error:
 
 static void bnxt_tx_int(struct bnxt *bp, struct bnxt_napi *bnapi, int nr_pkts)
 {
-   struct bnxt_tx_ring_info *txr = >tx_ring;
+   struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
int index = bnapi->index;
struct netdev_queue *txq = netdev_get_tx_queue(bp->dev, index);
u16 cons = txr->tx_cons;
@@ -602,7 +600,7 @@ static void bnxt_reuse_rx_agg_bufs(struct bnxt_napi *bnapi, 
u16 cp_cons,
 {
struct bnxt *bp = bnapi->bp;
struct bnxt_cp_ring_info *cpr = >cp_ring;
-   struct bnxt_rx_ring_info *rxr = >rx_ring;
+   struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
u16 prod = rxr->rx_agg_prod;
u16 sw_prod = rxr->rx_sw_agg_prod;
u32 i;
@@ -681,7 +679,7 @@ static struct sk_buff *bnxt_rx_pages(struct bnxt *bp, 
struct bnxt_napi *bnapi,
 {
struct pci_dev *pdev = bp->pdev;
struct bnxt_cp_ring_info *cpr = >cp_ring;
-   struct bnxt_rx_ring_info *rxr = >rx_ring;
+   struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
u16 prod = rxr->rx_agg_prod;
u32 i;
 
@@ -940,7 +938,7 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
   bool *agg_event)
 {
struct bnxt_cp_ring_info *cpr = >cp_ring;
-   struct bnxt_rx_ring_info *rxr = >rx_ring;
+   struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
u8 agg_id = TPA_END_AGG_ID(tpa_end);
u8 *data, agg_bufs;
u16 cp_cons = RING_CMP(*raw_cons);
@@ -1056,7 +1054,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi 
*bnapi, u32 *raw_cons,
   bool *agg_event)
 {
struct bnxt_cp_ring_info *cpr = >cp_ring;
-   struct bnxt_rx_ring_info *rxr = >rx_ring;
+   struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
struct net_device *dev = bp->dev;
struct rx_cmp *rxcmp;
struct rx_cmp_ext *rxcmp1;
@@ -1390,7 +1388,7 @@ static int bnxt_poll_work(struct bnxt *bp, struct 
bnxt_napi *bnapi, int budget)
bnxt_tx_int(bp, bnapi, tx_pkts);
 
if (rx_event) {
-   struct bnxt_rx_ring_info *rxr = >rx_ring;
+   struct bnxt_rx_ring_info *rxr = bnapi->rx_ring;
 
writel(DB_KEY_RX | rxr->rx_prod, rxr->rx_doorbell);
writel(DB_KEY_RX | rxr->rx_prod, rxr->rx_doorbell);
@@ -1459,19 +1457,14 @@ static void bnxt_free_tx_skbs(struct bnxt *bp)
int i, max_idx;
struct pci_dev *pdev = bp->pdev;
 
-   if (!bp->bnapi)
+   if (!bp->tx_ring)
return;
 
max_idx = bp->tx_nr_pages * TX_DESC_CNT;
for (i = 0; i < 

[PATCH net-next 5/7] bnxt_en: Modify bnxt_get_max_rings() to support shared or non shared rings.

[Michael Chan]

Add logic to calculate how many shared or non shared rings can be
supported.  Default is to use shared rings.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 96 +--
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  4 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  4 +-
 3 files changed, 79 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8a54dab..4f7b231 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4038,6 +4038,30 @@ static int bnxt_set_real_num_queues(struct bnxt *bp)
return rc;
 }
 
+static int bnxt_trim_rings(struct bnxt *bp, int *rx, int *tx, int max,
+  bool shared)
+{
+   int _rx = *rx, _tx = *tx;
+
+   if (shared) {
+   *rx = min_t(int, _rx, max);
+   *tx = min_t(int, _tx, max);
+   } else {
+   if (max < 2)
+   return -ENOMEM;
+
+   while (_rx + _tx > max) {
+   if (_rx > _tx && _rx > 1)
+   _rx--;
+   else if (_tx > 1)
+   _tx--;
+   }
+   *rx = _rx;
+   *tx = _tx;
+   }
+   return 0;
+}
+
 static int bnxt_setup_msix(struct bnxt *bp)
 {
struct msix_entry *msix_ent;
@@ -4068,8 +4092,11 @@ static int bnxt_setup_msix(struct bnxt *bp)
int tcs;
 
/* Trim rings based upon num of vectors allocated */
-   bp->rx_nr_rings = min_t(int, total_vecs, bp->rx_nr_rings);
-   bp->tx_nr_rings = min_t(int, total_vecs, bp->tx_nr_rings);
+   rc = bnxt_trim_rings(bp, >rx_nr_rings, >tx_nr_rings,
+total_vecs, true);
+   if (rc)
+   goto msix_setup_exit;
+
bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
tcs = netdev_get_num_tc(dev);
if (tcs > 1) {
@@ -5337,10 +5364,10 @@ static int bnxt_setup_tc(struct net_device *dev, u8 tc)
return 0;
 
if (tc) {
-   int max_rx_rings, max_tx_rings;
+   int max_rx_rings, max_tx_rings, rc;
 
-   bnxt_get_max_rings(bp, _rx_rings, _tx_rings);
-   if (bp->tx_nr_rings_per_tc * tc > max_tx_rings)
+   rc = bnxt_get_max_rings(bp, _rx_rings, _tx_rings, true);
+   if (rc || bp->tx_nr_rings_per_tc * tc > max_tx_rings)
return -ENOMEM;
}
 
@@ -5654,31 +5681,62 @@ static int bnxt_get_max_irq(struct pci_dev *pdev)
return (ctrl & PCI_MSIX_FLAGS_QSIZE) + 1;
 }
 
-void bnxt_get_max_rings(struct bnxt *bp, int *max_rx, int *max_tx)
+static void _bnxt_get_max_rings(struct bnxt *bp, int *max_rx, int *max_tx,
+   int *max_cp)
 {
-   int max_rings = 0, max_ring_grps = 0;
+   int max_ring_grps = 0;
 
if (BNXT_PF(bp)) {
*max_tx = bp->pf.max_tx_rings;
*max_rx = bp->pf.max_rx_rings;
-   max_rings = min_t(int, bp->pf.max_irqs, bp->pf.max_cp_rings);
-   max_rings = min_t(int, max_rings, bp->pf.max_stat_ctxs);
+   *max_cp = min_t(int, bp->pf.max_irqs, bp->pf.max_cp_rings);
+   *max_cp = min_t(int, *max_cp, bp->pf.max_stat_ctxs);
max_ring_grps = bp->pf.max_hw_ring_grps;
} else {
 #ifdef CONFIG_BNXT_SRIOV
*max_tx = bp->vf.max_tx_rings;
*max_rx = bp->vf.max_rx_rings;
-   max_rings = min_t(int, bp->vf.max_irqs, bp->vf.max_cp_rings);
-   max_rings = min_t(int, max_rings, bp->vf.max_stat_ctxs);
+   *max_cp = min_t(int, bp->vf.max_irqs, bp->vf.max_cp_rings);
+   *max_cp = min_t(int, *max_cp, bp->vf.max_stat_ctxs);
max_ring_grps = bp->vf.max_hw_ring_grps;
 #endif
}
if (bp->flags & BNXT_FLAG_AGG_RINGS)
*max_rx >>= 1;
-
-   *max_rx = min_t(int, *max_rx, max_rings);
*max_rx = min_t(int, *max_rx, max_ring_grps);
-   *max_tx = min_t(int, *max_tx, max_rings);
+}
+
+int bnxt_get_max_rings(struct bnxt *bp, int *max_rx, int *max_tx, bool shared)
+{
+   int rx, tx, cp;
+
+   _bnxt_get_max_rings(bp, , , );
+   if (!rx || !tx || !cp)
+   return -ENOMEM;
+
+   *max_rx = rx;
+   *max_tx = tx;
+   return bnxt_trim_rings(bp, max_rx, max_tx, cp, shared);
+}
+
+static int bnxt_set_dflt_rings(struct bnxt *bp)
+{
+   int dflt_rings, max_rx_rings, max_tx_rings, rc;
+   bool sh = true;
+
+   if (sh)
+   bp->flags |= BNXT_FLAG_SHARED_RINGS;
+   dflt_rings = netif_get_num_default_rss_queues();
+   rc = bnxt_get_max_rings(bp, _rx_rings, _tx_rings, sh);
+   if (rc)
+   return rc;
+ 

[PATCH net-next 7/7] bnxt_en: Modify ethtool -l|-L to support combined or rx/tx rings.

[Michael Chan]

The driver can support either all combined or all rx/tx rings.  The
default is combined, but the user can now select rx/tx rings.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 57 ++-
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 8ad1b6c..4f62c9f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -212,6 +212,9 @@ static void bnxt_get_channels(struct net_device *dev,
int max_rx_rings, max_tx_rings, tcs;
 
bnxt_get_max_rings(bp, _rx_rings, _tx_rings, true);
+   channel->max_combined = max_rx_rings;
+
+   bnxt_get_max_rings(bp, _rx_rings, _tx_rings, false);
tcs = netdev_get_num_tc(dev);
if (tcs > 1)
max_tx_rings /= tcs;
@@ -219,9 +222,12 @@ static void bnxt_get_channels(struct net_device *dev,
channel->max_rx = max_rx_rings;
channel->max_tx = max_tx_rings;
channel->max_other = 0;
-   channel->max_combined = 0;
-   channel->rx_count = bp->rx_nr_rings;
-   channel->tx_count = bp->tx_nr_rings_per_tc;
+   if (bp->flags & BNXT_FLAG_SHARED_RINGS) {
+   channel->combined_count = bp->rx_nr_rings;
+   } else {
+   channel->rx_count = bp->rx_nr_rings;
+   channel->tx_count = bp->tx_nr_rings_per_tc;
+   }
 }
 
 static int bnxt_set_channels(struct net_device *dev,
@@ -230,19 +236,35 @@ static int bnxt_set_channels(struct net_device *dev,
struct bnxt *bp = netdev_priv(dev);
int max_rx_rings, max_tx_rings, tcs;
u32 rc = 0;
+   bool sh = false;
 
-   if (channel->other_count || channel->combined_count ||
-   !channel->rx_count || !channel->tx_count)
+   if (channel->other_count)
return -EINVAL;
 
-   bnxt_get_max_rings(bp, _rx_rings, _tx_rings, true);
+   if (!channel->combined_count &&
+   (!channel->rx_count || !channel->tx_count))
+   return -EINVAL;
+
+   if (channel->combined_count &&
+   (channel->rx_count || channel->tx_count))
+   return -EINVAL;
+
+   if (channel->combined_count)
+   sh = true;
+
+   bnxt_get_max_rings(bp, _rx_rings, _tx_rings, sh);
+
tcs = netdev_get_num_tc(dev);
if (tcs > 1)
max_tx_rings /= tcs;
 
-   if (channel->rx_count > max_rx_rings ||
-   channel->tx_count > max_tx_rings)
-   return -EINVAL;
+   if (sh && (channel->combined_count > max_rx_rings ||
+  channel->combined_count > max_tx_rings))
+   return -ENOMEM;
+
+   if (!sh && (channel->rx_count > max_rx_rings ||
+   channel->tx_count > max_tx_rings))
+   return -ENOMEM;
 
if (netif_running(dev)) {
if (BNXT_PF(bp)) {
@@ -258,12 +280,23 @@ static int bnxt_set_channels(struct net_device *dev,
}
}
 
-   bp->rx_nr_rings = channel->rx_count;
-   bp->tx_nr_rings_per_tc = channel->tx_count;
+   if (sh) {
+   bp->flags |= BNXT_FLAG_SHARED_RINGS;
+   bp->rx_nr_rings = channel->combined_count;
+   bp->tx_nr_rings_per_tc = channel->combined_count;
+   } else {
+   bp->flags &= ~BNXT_FLAG_SHARED_RINGS;
+   bp->rx_nr_rings = channel->rx_count;
+   bp->tx_nr_rings_per_tc = channel->tx_count;
+   }
+
bp->tx_nr_rings = bp->tx_nr_rings_per_tc;
if (tcs > 1)
bp->tx_nr_rings = bp->tx_nr_rings_per_tc * tcs;
-   bp->cp_nr_rings = max_t(int, bp->tx_nr_rings, bp->rx_nr_rings);
+
+   bp->cp_nr_rings = sh ? max_t(int, bp->tx_nr_rings, bp->rx_nr_rings) :
+  bp->tx_nr_rings + bp->rx_nr_rings;
+
bp->num_stat_ctxs = bp->cp_nr_rings;
 
/* After changing number of rx channels, update NTUPLE feature. */
-- 
1.8.3.1

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

[PATCH net-next 0/7] bnxt_en: Support combined and rx/tx channels.

[Michael Chan]

The bnxt hardware uses a completion ring for rx and tx events.  The driver
has to process the completion ring entries sequentially for the events.
The current code only supports an rx/tx ring pair for each completion ring.
This patch series add support for using a dedicated completion ring for
rx only or tx only as an option configuarble using ethtool -L.

The benefits for using dedicated completion rings are:

1. A burst of rx packets can cause delay in processing tx events if the
completion ring is shared.  If tx queue is stopped by BQL, this can cause
delay in re-starting the tx queue.

2. A completion ring is sized according to the rx and tx ring size rounded
up to the nearest power of 2.  When the completion ring is shared, it is
sized by adding the rx and tx ring sizes and then rounded to the next power
of 2, often with a lot of wasted space.

3. Using dedicated completion ring, we can adjust the tx and rx coalescing
parameters independently for rx and tx.


Michael Chan (7):
  bnxt_en: Refactor bnxt_dbg_dump_states().
  bnxt_en: Separate bnxt_{rx|tx}_ring_info structs from bnxt_napi
struct.
  bnxt_en: Check for NULL rx or tx ring.
  bnxt_en: Re-structure ring indexing and mapping.
  bnxt_en: Modify bnxt_get_max_rings() to support shared or non shared
rings.
  bnxt_en: Modify init sequence to support shared or non shared rings.
  bnxt_en: Modify ethtool -l|-L to support combined or rx/tx rings.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 439 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h |  15 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c |  59 ++-
 3 files changed, 328 insertions(+), 185 deletions(-)

-- 
1.8.3.1

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

[PATCH net-next 6/7] bnxt_en: Modify init sequence to support shared or non shared rings.

[Michael Chan]

Modify ring memory allocation and MSIX setup to support shared or
non shared rings and do the proper mapping.  Default is still to
use shared rings.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 42 +++
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4f7b231..f956949 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2496,7 +2496,7 @@ static void bnxt_free_mem(struct bnxt *bp, bool 
irq_re_init)
 
 static int bnxt_alloc_mem(struct bnxt *bp, bool irq_re_init)
 {
-   int i, rc, size, arr_size;
+   int i, j, rc, size, arr_size;
void *bnapi;
 
if (irq_re_init) {
@@ -2535,9 +2535,14 @@ static int bnxt_alloc_mem(struct bnxt *bp, bool 
irq_re_init)
if (!bp->tx_ring)
return -ENOMEM;
 
-   for (i = 0; i < bp->tx_nr_rings; i++) {
-   bp->tx_ring[i].bnapi = bp->bnapi[i];
-   bp->bnapi[i]->tx_ring = >tx_ring[i];
+   if (bp->flags & BNXT_FLAG_SHARED_RINGS)
+   j = 0;
+   else
+   j = bp->rx_nr_rings;
+
+   for (i = 0; i < bp->tx_nr_rings; i++, j++) {
+   bp->tx_ring[i].bnapi = bp->bnapi[j];
+   bp->bnapi[j]->tx_ring = >tx_ring[i];
}
 
rc = bnxt_alloc_stats(bp);
@@ -4066,7 +4071,7 @@ static int bnxt_setup_msix(struct bnxt *bp)
 {
struct msix_entry *msix_ent;
struct net_device *dev = bp->dev;
-   int i, total_vecs, rc = 0;
+   int i, total_vecs, rc = 0, min = 1;
const int len = sizeof(bp->irq_tbl[0].name);
 
bp->flags &= ~BNXT_FLAG_USING_MSIX;
@@ -4081,7 +4086,10 @@ static int bnxt_setup_msix(struct bnxt *bp)
msix_ent[i].vector = 0;
}
 
-   total_vecs = pci_enable_msix_range(bp->pdev, msix_ent, 1, total_vecs);
+   if (!(bp->flags & BNXT_FLAG_SHARED_RINGS))
+   min = 2;
+
+   total_vecs = pci_enable_msix_range(bp->pdev, msix_ent, min, total_vecs);
if (total_vecs < 0) {
rc = -ENODEV;
goto msix_setup_exit;
@@ -4093,7 +4101,7 @@ static int bnxt_setup_msix(struct bnxt *bp)
 
/* Trim rings based upon num of vectors allocated */
rc = bnxt_trim_rings(bp, >rx_nr_rings, >tx_nr_rings,
-total_vecs, true);
+total_vecs, min == 1);
if (rc)
goto msix_setup_exit;
 
@@ -4115,12 +4123,21 @@ static int bnxt_setup_msix(struct bnxt *bp)
}
}
}
-   bp->cp_nr_rings = max_t(int, bp->rx_nr_rings, bp->tx_nr_rings);
+   bp->cp_nr_rings = total_vecs;
 
for (i = 0; i < bp->cp_nr_rings; i++) {
+   char *attr;
+
bp->irq_tbl[i].vector = msix_ent[i].vector;
+   if (bp->flags & BNXT_FLAG_SHARED_RINGS)
+   attr = "TxRx";
+   else if (i < bp->rx_nr_rings)
+   attr = "rx";
+   else
+   attr = "tx";
+
snprintf(bp->irq_tbl[i].name, len,
-"%s-%s-%d", dev->name, "TxRx", i);
+"%s-%s-%d", dev->name, attr, i);
bp->irq_tbl[i].handler = bnxt_msix;
}
rc = bnxt_set_real_num_queues(bp);
@@ -4158,6 +4175,7 @@ static int bnxt_setup_inta(struct bnxt *bp)
bp->tx_nr_rings = 1;
bp->cp_nr_rings = 1;
bp->tx_nr_rings_per_tc = bp->tx_nr_rings;
+   bp->flags |= BNXT_FLAG_SHARED_RINGS;
bp->irq_tbl[0].vector = bp->pdev->irq;
snprintf(bp->irq_tbl[0].name, len,
 "%s-%s-%d", bp->dev->name, "TxRx", 0);
@@ -5365,8 +5383,12 @@ static int bnxt_setup_tc(struct net_device *dev, u8 tc)
 
if (tc) {
int max_rx_rings, max_tx_rings, rc;
+   bool sh = false;
+
+   if (bp->flags & BNXT_FLAG_SHARED_RINGS)
+   sh = true;
 
-   rc = bnxt_get_max_rings(bp, _rx_rings, _tx_rings, true);
+   rc = bnxt_get_max_rings(bp, _rx_rings, _tx_rings, sh);
if (rc || bp->tx_nr_rings_per_tc * tc > max_tx_rings)
return -ENOMEM;
}
-- 
1.8.3.1

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

[PATCH net-next 1/7] bnxt_en: Refactor bnxt_dbg_dump_states().

[Michael Chan]

By adding 3 separate functions to dump the different ring states.

Signed-off-by: Michael Chan 
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 50 ---
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c 
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 319653a..3f053de 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -5015,31 +5015,47 @@ static int bnxt_set_features(struct net_device *dev, 
netdev_features_t features)
return rc;
 }
 
+static void bnxt_dump_tx_sw_state(struct bnxt_napi *bnapi)
+{
+   struct bnxt_tx_ring_info *txr = >tx_ring;
+   int i = bnapi->index;
+
+   netdev_info(bnapi->bp->dev, "[%d]: tx{fw_ring: %d prod: %x cons: %x}\n",
+   i, txr->tx_ring_struct.fw_ring_id, txr->tx_prod,
+   txr->tx_cons);
+}
+
+static void bnxt_dump_rx_sw_state(struct bnxt_napi *bnapi)
+{
+   struct bnxt_rx_ring_info *rxr = >rx_ring;
+   int i = bnapi->index;
+
+   netdev_info(bnapi->bp->dev, "[%d]: rx{fw_ring: %d prod: %x} 
rx_agg{fw_ring: %d agg_prod: %x sw_agg_prod: %x}\n",
+   i, rxr->rx_ring_struct.fw_ring_id, rxr->rx_prod,
+   rxr->rx_agg_ring_struct.fw_ring_id, rxr->rx_agg_prod,
+   rxr->rx_sw_agg_prod);
+}
+
+static void bnxt_dump_cp_sw_state(struct bnxt_napi *bnapi)
+{
+   struct bnxt_cp_ring_info *cpr = >cp_ring;
+   int i = bnapi->index;
+
+   netdev_info(bnapi->bp->dev, "[%d]: cp{fw_ring: %d raw_cons: %x}\n",
+   i, cpr->cp_ring_struct.fw_ring_id, cpr->cp_raw_cons);
+}
+
 static void bnxt_dbg_dump_states(struct bnxt *bp)
 {
int i;
struct bnxt_napi *bnapi;
-   struct bnxt_tx_ring_info *txr;
-   struct bnxt_rx_ring_info *rxr;
-   struct bnxt_cp_ring_info *cpr;
 
for (i = 0; i < bp->cp_nr_rings; i++) {
bnapi = bp->bnapi[i];
-   txr = >tx_ring;
-   rxr = >rx_ring;
-   cpr = >cp_ring;
if (netif_msg_drv(bp)) {
-   netdev_info(bp->dev, "[%d]: tx{fw_ring: %d prod: %x 
cons: %x}\n",
-   i, txr->tx_ring_struct.fw_ring_id,
-   txr->tx_prod, txr->tx_cons);
-   netdev_info(bp->dev, "[%d]: rx{fw_ring: %d prod: %x} 
rx_agg{fw_ring: %d agg_prod: %x sw_agg_prod: %x}\n",
-   i, rxr->rx_ring_struct.fw_ring_id,
-   rxr->rx_prod,
-   rxr->rx_agg_ring_struct.fw_ring_id,
-   rxr->rx_agg_prod, rxr->rx_sw_agg_prod);
-   netdev_info(bp->dev, "[%d]: cp{fw_ring: %d raw_cons: 
%x}\n",
-   i, cpr->cp_ring_struct.fw_ring_id,
-   cpr->cp_raw_cons);
+   bnxt_dump_tx_sw_state(bnapi);
+   bnxt_dump_rx_sw_state(bnapi);
+   bnxt_dump_cp_sw_state(bnapi);
}
}
 }
-- 
1.8.3.1

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