* Re: [PATCH 0/6] cxgb4: Fine-tuning for some function implementations
From: Ganesh GR @ 2017-05-09 9:40 UTC (permalink / raw)
To: SF Markus Elfring, netdev@vger.kernel.org, Casey Leedom
Cc: LKML, kernel-janitors@vger.kernel.org
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
Acked-by: Ganesh Goudar <ganeshgr@chelsio.com>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Sent: Friday, May 5, 2017 2:00 AM
To: netdev@vger.kernel.org; Casey Leedom; Ganesh GR
Cc: LKML; kernel-janitors@vger.kernel.org
Subject: [PATCH 0/6] cxgb4: Fine-tuning for some function implementations
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 22:23:45 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (6):
Use seq_putc() in mboxlog_show()
Combine substrings for 24 messages
Adjust five checks for null pointers
Replace seven seq_puts() calls by seq_putc()
Use seq_puts() in cim_qcfg_show()
Combine substrings for two messages
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 42 +++----
.../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 125 ++++++++++++---------
2 files changed, 86 insertions(+), 81 deletions(-)
--
2.12.2
^ permalink raw reply
* Re:Re: [PATCH net] driver: vrf: Fix one possible use-after-free issue
From: Gao Feng @ 2017-05-09 9:41 UTC (permalink / raw)
To: Florian Westphal; +Cc: dsa, shm, davem, netdev, gfree.wind@foxmail.com
In-Reply-To: <20170509092102.GB16263@breakpoint.cc>
At 2017-05-09 17:21:02, "Florian Westphal" <fw@strlen.de> wrote:
>gfree.wind@vip.163.com <gfree.wind@vip.163.com> wrote:
>> When one netfilter rule or hook stoles the skb and return NF_STOLEN,
>> it means the skb is taken by the rule, and other modules should not
>> touch this skb ever. Maybe the skb is queued or freed directly by the
>> rule.
>>
>> Now uses the nf_hook instead of NF_HOOK to get the result of netfilter,
>> and check the return value of nf_hook. Only when its value equals 1, it
>> means the skb could go ahead. Or reset the skb as NULL.
>>
>> BTW, because vrf_rcv_finish is empty function, so needn't invoke it
>> even though nf_hook returns 1.
>
>Thats a bug then.
>
>The okfn (if called) takes ownership of skb and must free it eventually.
>Otherwise userspace queueing leaks skb on reinjection.
>
>(see nf_reinject() and its use of okfn()).
Thanks, I only thought about the stolen case like synproxy which would free the skb directly,
and forget the userspace could reinject the skb.
I would update the patch.
Best Regards
Feng
^ permalink raw reply
* Re: [PATCH RFC net-next 0/6] net: reducing memory footprint of network devices
From: Nicolas Dichtel @ 2017-05-09 9:50 UTC (permalink / raw)
To: Florian Fainelli, David Ahern, netdev; +Cc: roopa
In-Reply-To: <89333e30-0a5d-0f05-4d14-d75dc157d6ba@gmail.com>
Le 08/05/2017 à 19:35, Florian Fainelli a écrit :
> On 05/06/2017 09:07 AM, David Ahern wrote:
>> As I have mentioned many times[1], at ~43+kB per instance the use of
>> net_devices does not scale for deployments needing 10,000+ devices. At
>> netconf 1.2 there was a discussion about using a net_device_common for
>> the minimal set of common attributes with other structs built on top of
>> that one for "full" devices. It provided a means for the code to know
>> "non-standard" net_devices. Conceptually, that approach has its merits
>> but it is not practical given the sweeping changes required to the code
>> base. More importantly though struct net_device is not the problem; it
>> weighs in at less than 2kB so reorganizing the code base around a
>> refactored net_device is not going to solve the problem. The primary
>> issue is all of the initializations done *because* it is a struct
>> net_device -- kobject and sysfs and the protocols (e.g., ipv4, ipv6,
>> mpls, neighbors).
>>
>> So, how do you keep the desired attributes of a net device -- network
>> addresses, xmit function, qdisc, netfilter rules, tcpdump -- while
>> lowering the overhead of a net_device instance and without sweeping
>> changes across net/ and drivers/net/?
>>
>> This patch set introduces the concept of labeling net_devices as
>> "lightweight", first mentioned at netdev 1.1 [1]. Users have to opt
>> in to lightweight devices by passing a new attribute, IFLA_LWT_NETDEV,
>> in the new link request. This lightweight tag is meant for virtual
>> devices such as vlan, vrf, vti, and dummy where the user expects to
>> create a lot of them and does not want the duplication of resources.
>> Each device type can always opt out of a lightweight label if necessary
>> by failing device creates.
>>
>> Labeling a virtual device as "lightweight" reduces the footprint for
>> device creation from ~43kB to ~6kB. That reduction in memory is obtained
>> by:
>> 1. no entry in sysfs
>> - kobject in net_device.device is not initialized
>>
>> 2. no entry in procfs
>> - no sysctl option for these devices
>>
>> 3. deferred ipv4, ipv6, mpls initialization
>> - network layer must be enabled before an address can be assigned
>> or mpls labels can be processed
>> - enables what Florian called L2 only devices [2]
>>
>> Once the core premise of a lightweight device is accepted, follow on
>> patches can reduce the overhead of network initializations. e.g.,
>>
>> 1. remove devconf per device (ipv4 and ipv6)
>> - lightweight devices use the default settings rather than replicate
>> the same data for each device
>>
>> 2. reduce / remove / opt out of snmp mibs
>> - snmp6_alloc_dev and icmpv6msg_mib_device specifically is a heavy
>> hitter
>>
>> Patches can also be found here:
>> https://github.com/dsahern/linux lwt-dev-rfc
>>
>> And iproute2 here:
>> https://github.com/dsahern/iproute2 lwt-dev
>>
>> Example:
>> ip li add foo lwd type vrf table 123
>>
>> - creates VRF device 'foo' as a lightweight netdevice.
>
> This is really looking nice, thanks for posting this patch series! The
> only submission wide comment I have is that the flag is named
> IFF_LWT_NETDEV whereas the helper that checks for it is named
> netif_is_lwd() so we should reconcile the two. Since there is an
> existing lightweight tunnel infrastructure already, maybe using
> IFF_LWD_NETDEV (or just IFF_LWD) would be good enough here?
Yep, thank you for the series, it also looks good to me.
I also vote for the IFF_LWD_NETDEV or IFF_LWD to avoid confusion with
lightweight tunnel and to be consistent with it (lightweight was abbreviated lw,
not lwt ;-)).
Your initial patch tried to make those interfaces transparent, this is not the
case anymore here. It would probably be useful to be able to filter those
interfaces in the kernel during a dump.
Regards,
Nicolas
^ permalink raw reply
* [PATCH net v2] driver: vrf: Fix one possible use-after-free issue
From: gfree.wind @ 2017-05-09 10:27 UTC (permalink / raw)
To: dsa, shm, davem, fw, netdev; +Cc: Gao Feng
From: Gao Feng <gfree.wind@vip.163.com>
The current codes only deal with the case that the skb is dropped, it
may meet one use-after-free issue when NF_HOOK returns 0 that means
the skb is stolen by one netfilter rule or hook.
When one netfilter rule or hook stoles the skb and return NF_STOLEN,
it means the skb is taken by the rule, and other modules should not
touch this skb ever. Maybe the skb is queued or freed directly by the
rule.
Now uses the nf_hook instead of NF_HOOK to get the result of netfilter,
and check the return value of nf_hook. Only when its value equals 1, it
means the skb could go ahead. Or reset the skb as NULL.
BTW, because vrf_rcv_finish is empty function, so needn't invoke it
even though nf_hook returns 1. But we need to modify vrf_rcv_finish
to deal with the NF_STOLEN case.
There are two cases when skb is stolen.
1. The skb is stolen and freed directly.
There is nothing we need to do, and vrf_rcv_finish isn't invoked.
2. The skb is queued and reinjected again.
The vrf_rcv_finish would be invoked as okfn, so need to free the
skb in it.
Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
v2: Free the skb in vrf_rcv_finish, per Florian
v1: initial version
drivers/net/vrf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index ceda586..db88249 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -989,6 +989,7 @@ static u32 vrf_fib_table(const struct net_device *dev)
static int vrf_rcv_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
{
+ kfree_skb(skb);
return 0;
}
@@ -998,7 +999,7 @@ static struct sk_buff *vrf_rcv_nfhook(u8 pf, unsigned int hook,
{
struct net *net = dev_net(dev);
- if (NF_HOOK(pf, hook, net, NULL, skb, dev, NULL, vrf_rcv_finish) < 0)
+ if (nf_hook(pf, hook, net, NULL, skb, dev, NULL, vrf_rcv_finish) != 1)
skb = NULL; /* kfree_skb(skb) handled by nf code */
return skb;
--
1.9.1
^ permalink raw reply related
* 3590
From: kirola @ 2017-05-09 10:29 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 81646126599.zip --]
[-- Type: application/zip, Size: 1886 bytes --]
^ permalink raw reply
* RE: FEC on i.MX 7 transmit queue timeout
From: Andy Duan @ 2017-05-09 10:35 UTC (permalink / raw)
To: Stefan Agner, Andrew Lunn
Cc: festevam@gmail.com, netdev@vger.kernel.org,
netdev-owner@vger.kernel.org
In-Reply-To: <d48a6d17b05280331642ea683caabc81@agner.ch>
From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, May 09, 2017 2:23 AM
>To: Andy Duan <fugang.duan@nxp.com>; Andrew Lunn <andrew@lunn.ch>
>Cc: festevam@gmail.com; netdev@vger.kernel.org; netdev-
>owner@vger.kernel.org
>Subject: Re: FEC on i.MX 7 transmit queue timeout
>
>On 2017-05-07 19:13, Andy Duan wrote:
>> From: Andrew Lunn <andrew@lunn.ch> Sent: Friday, May 05, 2017 8:24 PM
>>>To: Andy Duan <fugang.duan@nxp.com>
>>>Cc: Stefan Agner <stefan@agner.ch>; festevam@gmail.com;
>>>netdev@vger.kernel.org; netdev-owner@vger.kernel.org
>>>Subject: Re: FEC on i.MX 7 transmit queue timeout
>>>
>>>> No, it is not workaround. As i said, quque1 and queue2 are for AVB
>>>> paths have higher priority in transmition.
>>>
>>>Does this higher priority result in the low priority queue being
>>>starved? Is that why the timer goes off? What happens when somebody
>>>does use AVB. Are we back to the same problem? This is what seems to
>>>make is sounds like a work around, not a fix.
>>>
>>> Andrew
>> Yes, queue0 may be blocked by queue1 and queue2, then the queue0
>> watchdog time maybe triggered.
>> If somebody use AVB quque1 and queue2, the remaining bandwidth is for
>> queue0, for example, in 100Mbps system, quque1 cost 50Mbps bandwidth
>> and queue2 cost 50Mbps bandwidth for audio and video streaming, then
>> queue0 (best effort) has 0 bandwidth that limit user case cannot have
>> asynchronous frames (IP(tcp/udp)) on networking. Of course these is
>> extreme case.
>> In essentially, asynchronous frames (IP) go queue0 for the original
>> design. To do these just implement .ndo_select_queue() callback in
>> driver like fsl tree.
>
>I guess you refer to this commit?
>
>http://git.freescale.com/git/cgit.cgi/imx/linux-
>imx.git/commit/?h=imx_4.1.15_2.0.0_ga&id=b0d8fa989651baf847287f6888f4d
>7b723e2a207
>
>It seems that by default there is a Credit-based scheme enabled
>(ENETx_QOS[TX_SCHEME] = 000). The driver enables the queue1/2 and
>assigns it each 50% of the bandwidth (IDLE_SLOPE_1/2 is set to 0x200, which
>according to the register description of IDLE_SLOPE means BW fraction of 0.5!).
>This actually violates even the note in register
>ENETx_DMAnCFG:
>
>"NOTE: For AVB applications, the BW fraction of class 1 and class 2 combined
>must not exceed .75."
>
Yes, the suggested BW fraction for class1 and class2 must not exceed .75, otherwise best effort
Queue will be blocked then watchdog timeout on queue0.
How to set the BW fraction that is up to user case.
>Instead of using the credit based scheme we could switch to round robin, but
>not sure if that is what we want.
>
Using round robin has no meaning for AVB feature.
>What is the default criteria to select queues when .ndo_select_queue is not
>provided? I guess it tries to balance individual streams/processes for better
>SMP performance?
>
Since AVB audio/video streaming is based on IEEE1722 format, .ndo_select_queue just parse
the avb streaming and insect the skb into class1 and class2 queues, which are individual streaming and processes.
^ permalink raw reply
* Re: [PATCH] net: wireless: ath: ath9k: remove unnecessary code
From: Kalle Valo @ 2017-05-09 11:32 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: ath9k-devel, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170509034814.GA18034@embeddedgus>
"Gustavo A. R. Silva" <garsilva@embeddedor.com> writes:
> The name of an array used by itself will always return the array's address.
> So this test will always evaluate as true.
>
> Addresses-Coverity-ID: 1364903
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
> drivers/net/wireless/ath/ath9k/eeprom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
> index fb80ec8..5c3bc28 100644
> --- a/drivers/net/wireless/ath/ath9k/eeprom.c
> +++ b/drivers/net/wireless/ath/ath9k/eeprom.c
> @@ -143,7 +143,7 @@ bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data)
>
> if (ah->eeprom_blob)
> ret = ath9k_hw_nvram_read_firmware(ah->eeprom_blob, off, data);
> - else if (pdata && !pdata->use_eeprom && pdata->eeprom_data)
> + else if (pdata && !pdata->use_eeprom)
> ret = ath9k_hw_nvram_read_pdata(pdata, off, data);
> else
> ret = common->bus_ops->eeprom_read(common, off, data);
The patch may very well be valid (didn't check yet) but the commit log
is gibberish for me.
--
Kalle Valo
^ permalink raw reply
* [PATCH net 0/3] mlx4 misc fixes
From: Tariq Toukan @ 2017-05-09 11:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Tariq Toukan
Hi Dave,
This patchset contains misc bug fixes from the team
to the mlx4 Core and Eth drivers.
Series generated against net commit:
32f1bc0f3d26 Revert "ipv4: restore rt->fi for reference counting"
Thanks,
Tariq.
Jack Morgenstein (1):
net/mlx4_core: Reduce harmless SRIOV error message to debug level
Kamal Heib (1):
net/mlx4_en: Change the error print to debug print
Talat Batheesh (1):
net/mlx4_en: Avoid adding steering rules with invalid ring
drivers/net/ethernet/mellanox/mlx4/cmd.c | 14 +++++++++++---
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 5 +++++
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 ++-
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 2 +-
4 files changed, 19 insertions(+), 5 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net 2/3] net/mlx4_en: Avoid adding steering rules with invalid ring
From: Tariq Toukan @ 2017-05-09 11:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Talat Batheesh, Tariq Toukan
In-Reply-To: <1494330324-11752-1-git-send-email-tariqt@mellanox.com>
From: Talat Batheesh <talatb@mellanox.com>
Inserting steering rules with illegal ring is an invalid operation,
block it.
Fixes: 820672812f82 ('net/mlx4_en: Manage flow steering rules with ethtool')
Signed-off-by: Talat Batheesh <talatb@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index ffbcb27c05e5..ae5fdc2df654 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -1562,6 +1562,11 @@ static int mlx4_en_flow_replace(struct net_device *dev,
qpn = priv->drop_qp.qpn;
else if (cmd->fs.ring_cookie & EN_ETHTOOL_QP_ATTACH) {
qpn = cmd->fs.ring_cookie & (EN_ETHTOOL_QP_ATTACH - 1);
+ if (qpn < priv->rss_map.base_qpn ||
+ qpn >= priv->rss_map.base_qpn + priv->rx_ring_num) {
+ en_warn(priv, "rxnfc: QP (0x%x) doesn't exist\n", qpn);
+ return -EINVAL;
+ }
} else {
if (cmd->fs.ring_cookie >= priv->rx_ring_num) {
en_warn(priv, "rxnfc: RX ring (%llu) doesn't exist\n",
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 1/3] net/mlx4_en: Change the error print to debug print
From: Tariq Toukan @ 2017-05-09 11:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Kamal Heib, Tariq Toukan
In-Reply-To: <1494330324-11752-1-git-send-email-tariqt@mellanox.com>
From: Kamal Heib <kamalh@mellanox.com>
The error print within mlx4_en_calc_rx_buf() should be a debug print.
Fixes: 51151a16a60f ('mlx4: allow order-0 memory allocations in RX path')
Signed-off-by: Kamal Heib <kamalh@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index aa074e57ce06..77abd1813047 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -997,7 +997,8 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
en_dbg(DRV, priv, "Rx buffer scatter-list (effective-mtu:%d num_frags:%d):\n",
eff_mtu, priv->num_frags);
for (i = 0; i < priv->num_frags; i++) {
- en_err(priv,
+ en_dbg(DRV,
+ priv,
" frag:%d - size:%d stride:%d\n",
i,
priv->frag_info[i].frag_size,
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 3/3] net/mlx4_core: Reduce harmless SRIOV error message to debug level
From: Tariq Toukan @ 2017-05-09 11:45 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Jack Morgenstein, Tariq Toukan
In-Reply-To: <1494330324-11752-1-git-send-email-tariqt@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
Under SRIOV resource management, extra counters are allocated to VFs
from a free pool. If that pool is empty, the ALLOC_RES command for
a counter resource fails -- and this generates a misleading error
message in the message log.
Under SRIOV, each VF is allocated (i.e., guaranteed) 2 counters --
one counter per port. For ETH ports, the RoCE driver requests an
additional counter (above the guaranteed counters). If that request
fails, the VF RoCE driver simply uses the default (i.e., guaranteed)
counter for that port.
Thus, failing to allocate an additional counter does not constitute
a problem, and the error message on the PF when this occurs should
be reduced to debug level.
Finally, to identify the situation that the reason for the failure is
that no resources are available to grant to the VF, we modified the
error returned by mlx4_grant_resource to -EDQUOT (Quota exceeded),
which more accurately describes the error.
Fixes: c3abb51bdb0e ("IB/mlx4: Add RoCE/IB dedicated counters")
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/cmd.c | 14 +++++++++++---
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 2 +-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 0e0fa7030565..c1af47e45d3f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -1789,9 +1789,17 @@ static int mlx4_master_process_vhcr(struct mlx4_dev *dev, int slave,
}
if (err) {
- if (!(dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR))
- mlx4_warn(dev, "vhcr command:0x%x slave:%d failed with error:%d, status %d\n",
- vhcr->op, slave, vhcr->errno, err);
+ if (!(dev->persist->state & MLX4_DEVICE_STATE_INTERNAL_ERROR)) {
+ if (vhcr->op == MLX4_CMD_ALLOC_RES &&
+ (vhcr->in_modifier & 0xff) == RES_COUNTER &&
+ err == -EDQUOT)
+ mlx4_dbg(dev,
+ "Unable to allocate counter for slave %d (%d)\n",
+ slave, err);
+ else
+ mlx4_warn(dev, "vhcr command:0x%x slave:%d failed with error:%d, status %d\n",
+ vhcr->op, slave, vhcr->errno, err);
+ }
vhcr_cmd->status = mlx4_errno_to_status(err);
goto out_status;
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 4aa29ee93013..07516545474f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -311,7 +311,7 @@ static inline int mlx4_grant_resource(struct mlx4_dev *dev, int slave,
struct mlx4_priv *priv = mlx4_priv(dev);
struct resource_allocator *res_alloc =
&priv->mfunc.master.res_tracker.res_alloc[res_type];
- int err = -EINVAL;
+ int err = -EDQUOT;
int allocated, free, reserved, guaranteed, from_free;
int from_rsvd;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/4] net: stmmac: Fine-tuning for four function implementations
From: SF Markus Elfring @ 2017-05-09 11:51 UTC (permalink / raw)
To: netdev, Alexandre Torgue, Giuseppe Cavallaro; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 13:48:03 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (4):
Combine three seq_printf() calls into a seq_puts() in stmmac_sysfs_dma_cap_read()
Replace five seq_printf() calls by seq_puts()
Use seq_putc() in sysfs_display_ring()
Delete an unnecessary return statement in stmmac_get_tx_hwtstamp()
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH 1/4] net: stmmac: Combine three seq_printf() calls into a seq_puts() in stmmac_sysfs_dma_cap_read()
From: SF Markus Elfring @ 2017-05-09 11:52 UTC (permalink / raw)
To: netdev, Alexandre Torgue, Giuseppe Cavallaro; +Cc: LKML, kernel-janitors
In-Reply-To: <aa58027a-a39c-963e-4376-a4d5312ee118@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 13:10:28 +0200
A bit of text was put into a sequence by three separate function calls.
Print the same data by a single function call instead.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index cd8c60132390..2949c9fc18fa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3802,10 +3802,10 @@ static int stmmac_sysfs_dma_cap_read(struct seq_file *seq, void *v)
return 0;
}
- seq_printf(seq, "==============================\n");
- seq_printf(seq, "\tDMA HW features\n");
- seq_printf(seq, "==============================\n");
-
+ seq_puts(seq,
+ "==============================\n"
+ "\tDMA HW features\n"
+ "==============================\n");
seq_printf(seq, "\t10/100 Mbps: %s\n",
(priv->dma_cap.mbps_10_100) ? "Y" : "N");
seq_printf(seq, "\t1000 Mbps: %s\n",
--
2.12.2
^ permalink raw reply related
* [PATCH 2/4] net: stmmac: Replace five seq_printf() calls by seq_puts()
From: SF Markus Elfring @ 2017-05-09 11:53 UTC (permalink / raw)
To: netdev, Alexandre Torgue, Giuseppe Cavallaro; +Cc: LKML, kernel-janitors
In-Reply-To: <aa58027a-a39c-963e-4376-a4d5312ee118@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 13:20:03 +0200
Five strings which did not contain data format specifications should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2949c9fc18fa..1f7022ce78b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3748,11 +3748,11 @@ static int stmmac_sysfs_ring_read(struct seq_file *seq, void *v)
seq_printf(seq, "RX Queue %d:\n", queue);
if (priv->extend_desc) {
- seq_printf(seq, "Extended descriptor ring:\n");
+ seq_puts(seq, "Extended descriptor ring:\n");
sysfs_display_ring((void *)rx_q->dma_erx,
DMA_RX_SIZE, 1, seq);
} else {
- seq_printf(seq, "Descriptor ring:\n");
+ seq_puts(seq, "Descriptor ring:\n");
sysfs_display_ring((void *)rx_q->dma_rx,
DMA_RX_SIZE, 0, seq);
}
@@ -3764,11 +3764,11 @@ static int stmmac_sysfs_ring_read(struct seq_file *seq, void *v)
seq_printf(seq, "TX Queue %d:\n", queue);
if (priv->extend_desc) {
- seq_printf(seq, "Extended descriptor ring:\n");
+ seq_puts(seq, "Extended descriptor ring:\n");
sysfs_display_ring((void *)tx_q->dma_etx,
DMA_TX_SIZE, 1, seq);
} else {
- seq_printf(seq, "Descriptor ring:\n");
+ seq_puts(seq, "Descriptor ring:\n");
sysfs_display_ring((void *)tx_q->dma_tx,
DMA_TX_SIZE, 0, seq);
}
@@ -3798,7 +3798,7 @@ static int stmmac_sysfs_dma_cap_read(struct seq_file *seq, void *v)
struct stmmac_priv *priv = netdev_priv(dev);
if (!priv->hw_cap_support) {
- seq_printf(seq, "DMA HW features not supported\n");
+ seq_puts(seq, "DMA HW features not supported\n");
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH 3/4] net: stmmac: Use seq_putc() in sysfs_display_ring()
From: SF Markus Elfring @ 2017-05-09 11:54 UTC (permalink / raw)
To: netdev, Alexandre Torgue, Giuseppe Cavallaro; +Cc: LKML, kernel-janitors
In-Reply-To: <aa58027a-a39c-963e-4376-a4d5312ee118@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 13:24:27 +0200
A single character (line break) should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1f7022ce78b7..4fa95234978c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3730,7 +3730,7 @@ static void sysfs_display_ring(void *head, int size, int extend_desc,
le32_to_cpu(p->des2), le32_to_cpu(p->des3));
p++;
}
- seq_printf(seq, "\n");
+ seq_putc(seq, '\n');
}
}
--
2.12.2
^ permalink raw reply related
* [PATCH 4/4] net: stmmac: Delete an unnecessary return statement in stmmac_get_tx_hwtstamp()
From: SF Markus Elfring @ 2017-05-09 11:55 UTC (permalink / raw)
To: netdev, Alexandre Torgue, Giuseppe Cavallaro; +Cc: LKML, kernel-janitors
In-Reply-To: <aa58027a-a39c-963e-4376-a4d5312ee118@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 13:36:04 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: void function return statements are not generally useful
Thus remove such a statement in the affected function.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4fa95234978c..9ab4cbfa67f1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -445,8 +445,6 @@ static void stmmac_get_tx_hwtstamp(struct stmmac_priv *priv,
/* pass tstamp to stack */
skb_tstamp_tx(skb, &shhwtstamp);
}
-
- return;
}
/* stmmac_get_rx_hwtstamp - get HW RX timestamps
--
2.12.2
^ permalink raw reply related
* Re: [PATCH] net: wireless: ath: ath10k: remove unnecessary code
From: Gustavo A. R. Silva @ 2017-05-09 12:00 UTC (permalink / raw)
To: Kalle Valo
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <87lgq6387s.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
Hi Kalle,
Quoting Kalle Valo <kvalo-A+ZNKFmMK5xy9aJCnZT0Uw@public.gmane.org>:
> "Gustavo A. R. Silva" <garsilva-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org> writes:
>
>> The name of an array used by itself will always return the array's address.
>> So these tests will always evaluate as false and therefore the _return_
>> will never be executed.
>>
>> Signed-off-by: Gustavo A. R. Silva <garsilva-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>
> I don't understand the commit log, especially what does "The name of an
> array used by itself" mean?
>
Let me correct that and I'll send the patch again.
Thanks!
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [PATCH] net: wireless: ath: ath9k: remove unnecessary code
From: Gustavo A. R. Silva @ 2017-05-09 12:01 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath9k-devel, linux-wireless, netdev, linux-kernel
In-Reply-To: <87efvy1d07.fsf@kamboji.qca.qualcomm.com>
Hi Kalle,
Quoting Kalle Valo <kvalo@qca.qualcomm.com>:
> "Gustavo A. R. Silva" <garsilva@embeddedor.com> writes:
>
>> The name of an array used by itself will always return the array's address.
>> So this test will always evaluate as true.
>>
>> Addresses-Coverity-ID: 1364903
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> drivers/net/wireless/ath/ath9k/eeprom.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c
>> b/drivers/net/wireless/ath/ath9k/eeprom.c
>> index fb80ec8..5c3bc28 100644
>> --- a/drivers/net/wireless/ath/ath9k/eeprom.c
>> +++ b/drivers/net/wireless/ath/ath9k/eeprom.c
>> @@ -143,7 +143,7 @@ bool ath9k_hw_nvram_read(struct ath_hw *ah, u32
>> off, u16 *data)
>>
>> if (ah->eeprom_blob)
>> ret = ath9k_hw_nvram_read_firmware(ah->eeprom_blob, off, data);
>> - else if (pdata && !pdata->use_eeprom && pdata->eeprom_data)
>> + else if (pdata && !pdata->use_eeprom)
>> ret = ath9k_hw_nvram_read_pdata(pdata, off, data);
>> else
>> ret = common->bus_ops->eeprom_read(common, off, data);
>
> The patch may very well be valid (didn't check yet) but the commit log
> is gibberish for me.
>
Let me correct that and I'll send the patch again.
Thanks!
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [PATCH 1/4] net: macb: Add support for PTP timestamps in DMA descriptors
From: Richard Cochran @ 2017-05-09 12:04 UTC (permalink / raw)
To: Rafal Ozieblo
Cc: David Miller, nicolas.ferre, netdev, linux-kernel, devicetree,
linux-arm-kernel, harini.katakam, andrei.pistirica
In-Reply-To: <1494321885-14384-1-git-send-email-rafalo@cadence.com>
On Tue, May 09, 2017 at 10:24:45AM +0100, Rafal Ozieblo wrote:
> This patch adds support for PTP timestamps in
> DMA buffer descriptors. It checks capability at runtime
> and uses appropriate buffer descriptor.
>
> Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
You posted this series once before, on April 13, 2017. That makes
this v2. Please add v2 in the subject line, eg. [PATCH v2 1/4].
Also, add a cover letter [0/4] that summarizes the changes between v1
and v2 of the series.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH] net: wireless: ath: ath10k: remove unnecessary code
From: Arend Van Spriel @ 2017-05-09 12:07 UTC (permalink / raw)
To: Kalle Valo, Gustavo A. R. Silva
Cc: netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <87lgq6387s.fsf@kamboji.qca.qualcomm.com>
On 9-5-2017 7:33, Kalle Valo wrote:
> "Gustavo A. R. Silva" <garsilva@embeddedor.com> writes:
>
>> The name of an array used by itself will always return the array's address.
>> So these tests will always evaluate as false and therefore the _return_
>> will never be executed.
>>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>
> I don't understand the commit log, especially what does "The name of an
> array used by itself" mean?
The array fields in struct wmi_start_scan_arg that are checked here are
fixed size arrays so they can never be NULL.
Maybe that helps rephrasing this commit message.
Regards,
Arend
^ permalink raw reply
* [PATCH net 0/5] qed*: General fixes
From: Yuval Mintz @ 2017-05-09 12:07 UTC (permalink / raw)
To: davem, netdev; +Cc: Yuval Mintz
This series contain several fixes for qed and qede.
- #1 [and ~#5] relate to XDP cleanups
- #2 and #5 correct VF behavior
- #3 and #4 fix and add missing configurations needed for RoCE & storage
Dave,
Please consider applying the series to 'net'.
Thanks,
Yuval
Sudarsana Reddy Kalluru (1):
qede: Fix XDP memory leak on unload
Ram Amrani (1):
qed: Correct doorbell configuration for !4Kb pages
Yuval Mintz (3):
qed: Fix VF removal sequence
qed: Tell QM the number of tasks
qede: Split PF/VF ndos
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 1 +
drivers/net/ethernet/qlogic/qed/qed_dev.c | 2 +-
drivers/net/ethernet/qlogic/qed/qed_main.c | 6 ++++--
drivers/net/ethernet/qlogic/qede/qede_filter.c | 5 -----
drivers/net/ethernet/qlogic/qede/qede_main.c | 25 ++++++++++++++++++++++++-
5 files changed, 30 insertions(+), 9 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH net 2/5] qed: Fix VF removal sequence
From: Yuval Mintz @ 2017-05-09 12:07 UTC (permalink / raw)
To: davem, netdev; +Cc: Yuval Mintz
In-Reply-To: <1494331671-16273-1-git-send-email-Yuval.Mintz@cavium.com>
After previos changes in HW-stop scheme, VFs stopped sending CLOSE
messages to their PFs when they unload.
Fixes: 1226337ad98f ("qed: Correct HW stop flow")
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index b7ad36b..0cbbd59 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1093,10 +1093,12 @@ static int qed_slowpath_stop(struct qed_dev *cdev)
qed_free_stream_mem(cdev);
if (IS_QED_ETH_IF(cdev))
qed_sriov_disable(cdev, true);
+ }
+
+ qed_nic_stop(cdev);
- qed_nic_stop(cdev);
+ if (IS_PF(cdev))
qed_slowpath_irq_free(cdev);
- }
qed_disable_msix(cdev);
--
1.9.3
^ permalink raw reply related
* [PATCH net 1/5] qede: Fix XDP memory leak on unload
From: Yuval Mintz @ 2017-05-09 12:07 UTC (permalink / raw)
To: davem, netdev; +Cc: Suddarsana Reddy Kalluru, Yuval Mintz
In-Reply-To: <1494331671-16273-1-git-send-email-Yuval.Mintz@cavium.com>
From: Suddarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
When (re|un)loading, Tx-queues belonging to XDP would not get freed.
Fixes: cb6aeb079294 ("qede: Add support for XDP_TX")
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
drivers/net/ethernet/qlogic/qede/qede_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index b9ba23d..263fd28 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -1313,6 +1313,9 @@ static void qede_free_mem_fp(struct qede_dev *edev, struct qede_fastpath *fp)
if (fp->type & QEDE_FASTPATH_RX)
qede_free_mem_rxq(edev, fp->rxq);
+ if (fp->type & QEDE_FASTPATH_XDP)
+ qede_free_mem_txq(edev, fp->xdp_tx);
+
if (fp->type & QEDE_FASTPATH_TX)
qede_free_mem_txq(edev, fp->txq);
}
--
1.9.3
^ permalink raw reply related
* [PATCH net 3/5] qed: Tell QM the number of tasks
From: Yuval Mintz @ 2017-05-09 12:07 UTC (permalink / raw)
To: davem, netdev; +Cc: Yuval Mintz
In-Reply-To: <1494331671-16273-1-git-send-email-Yuval.Mintz@cavium.com>
Driver doesn't pass the number of tasks to the QM init logic
which would cause back-pressure in scenarios requiring many tasks
[E.g., using max MRs] and thus reduced performance.
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index b3aaa98..6948457 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -1460,6 +1460,7 @@ void qed_qm_init_pf(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
params.is_first_pf = p_hwfn->first_on_engine;
params.num_pf_cids = iids.cids;
params.num_vf_cids = iids.vf_cids;
+ params.num_tids = iids.tids;
params.start_pq = qm_info->start_pq;
params.num_pf_pqs = qm_info->num_pqs - qm_info->num_vf_pqs;
params.num_vf_pqs = qm_info->num_vf_pqs;
--
1.9.3
^ permalink raw reply related
* [PATCH net 4/5] qed: Correct doorbell configuration for !4Kb pages
From: Yuval Mintz @ 2017-05-09 12:07 UTC (permalink / raw)
To: davem, netdev; +Cc: Ram Amrani, Yuval Mintz
In-Reply-To: <1494331671-16273-1-git-send-email-Yuval.Mintz@cavium.com>
From: Ram Amrani <Ram.Amrani@cavium.com>
When configuring the doorbell DPI address, driver aligns the start
address to 4KB [HW-pages] instead of host PAGE_SIZE.
As a result, RoCE applications might receive addresses which are
unaligned to pages [when PAGE_SIZE > 4KB], which is a security risk.
Fixes: 51ff17251c9c ("qed: Add support for RoCE hw init")
Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index bb70522..463927f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -1370,7 +1370,7 @@ enum QED_ROCE_EDPM_MODE {
NULL) +
qed_cxt_get_proto_cid_count(p_hwfn, PROTOCOLID_ETH,
NULL);
- norm_regsize = roundup(QED_PF_DEMS_SIZE * non_pwm_conn, 4096);
+ norm_regsize = roundup(QED_PF_DEMS_SIZE * non_pwm_conn, PAGE_SIZE);
min_addr_reg1 = norm_regsize / 4096;
pwm_regsize = db_bar_size - norm_regsize;
--
1.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox