* Re: [PATCH 0/8] Fix and complete CAN namespace support
From: Oliver Hartkopp @ 2017-04-25 8:48 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can, davem; +Cc: dev, netdev
In-Reply-To: <9a88aee4-ba8c-00e7-2525-e300d905e593@pengutronix.de>
On 04/25/2017 10:45 AM, Marc Kleine-Budde wrote:
> FYI:
> This series is included in the latest pull request:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
> tags/linux-can-next-for-4.12-20170425
>
Ok, thanks!
Sorry for pushing a bit harder this time. But I wanted to make sure that
we don't get incomplete stuff into the merge window.
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH 0/8] Fix and complete CAN namespace support
From: Marc Kleine-Budde @ 2017-04-25 8:45 UTC (permalink / raw)
To: Oliver Hartkopp, linux-can, davem; +Cc: dev, netdev
In-Reply-To: <8fc17f07-f165-970b-f96a-1d920302fd00@pengutronix.de>
[-- Attachment #1.1: Type: text/plain, Size: 1105 bytes --]
On 04/25/2017 08:48 AM, Marc Kleine-Budde wrote:
> On 04/25/2017 08:19 AM, Oliver Hartkopp wrote:
>> Hello Dave,
>>
>> unfortunately the initial network namespace support by Mario Kicherer
>> (8e8cda6d737d) slipped into net-next without further review and Marc pushed
>> the code without my Acked-by. Due to the fact that this code was in net-next
>> now I spent some nights to fix, clean up, finalize and test the missing pieces
>> for the namespace support for the CAN subsystem in net/can.
>>
>> As Marc is currently *VERY* unresponsive on the mailing list due to his 'real'
>
> ... and holidays :) But I'm back in the office now.
FYI:
This series is included in the latest pull request:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
tags/linux-can-next-for-4.12-20170425
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* pull-request: can-next 2017-04-25
From: Marc Kleine-Budde @ 2017-04-25 8:44 UTC (permalink / raw)
To: netdev; +Cc: David Miller, kernel@pengutronix.de, linux-can@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 4994 bytes --]
Hello David,
this is a pull request of 21 patches for net-next/master.
There are 4 patches by Stephane Grosjean for the PEAK PCAN-PCIe FD
CAN-FD boards. The next 7 patches are by Mario Huettel, which add
support for M_CAN IP version >= v3.1.x to the m_can driver. A patch by
Remigiusz Kołłątaj adds support for the Microchip CAN BUS Analyzer. 8
patches by Oliver Hartkopp complete the initial CAN network namespace
support. Wei Yongjun's patch for the ti_hecc driver fixes the return
value check in the probe function.
Marc
---
The following changes since commit 14933dc8d9964e46f1d5bd2a4dfe3d3be8e420e0:
sparc64: Improve 64-bit constant loading in eBPF JIT. (2017-04-24 20:32:15 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-4.12-20170425
for you to fetch changes up to b655f0e96d4061eac42dea2dccd37a3348d1f3f3:
can: ti_hecc: fix return value check in ti_hecc_probe() (2017-04-25 10:03:40 +0200)
----------------------------------------------------------------
linux-can-next-for-4.12-20170425
----------------------------------------------------------------
Mario Huettel (7):
can: m_can: Disabled Interrupt Line 1
can: m_can: Removed initialization of FIFO water marks
can: m_can: Removed virtual address from print
can: m_can: Updated register defines to newest version
can: m_can: Enable M_CAN version dependent initialization
can: m_can: Configuration for TX and TX event FIFOs
can: m_can: Enable TX FIFO Handling for M_CAN IP version >= v3.1.x
Oliver Hartkopp (8):
can: fix memory leak in initial namespace support
can: remove obsolete pernet_operations definitions
can: remove obsolete definitions
can: complete initial namespace support
can: network namespace support for CAN_BCM protocol
can: network namespace support for CAN gateway
can: add Virtual CAN Tunnel driver (vxcan)
can: enable module auto loading for virtual CAN interfaces
Remigiusz Kołłątaj (1):
can: mcba_usb: Add support for Microchip CAN BUS Analyzer
Stephane Grosjean (4):
can: peak: fix usage of usb specific data type
can: peak: fix usage of const qualifier in pointers args
can: peak: move header file to new can common subdir
can: peak: add support for PEAK PCAN-PCIe FD CAN-FD boards
Wei Yongjun (1):
can: ti_hecc: fix return value check in ti_hecc_probe()
drivers/net/can/Kconfig | 19 +
drivers/net/can/Makefile | 2 +
drivers/net/can/m_can/m_can.c | 752 +++++++++++++----
drivers/net/can/peak_canfd/Kconfig | 13 +
drivers/net/can/peak_canfd/Makefile | 5 +
drivers/net/can/peak_canfd/peak_canfd.c | 801 ++++++++++++++++++
drivers/net/can/peak_canfd/peak_canfd_user.h | 55 ++
drivers/net/can/peak_canfd/peak_pciefd_main.c | 842 +++++++++++++++++++
drivers/net/can/ti_hecc.c | 12 +-
drivers/net/can/usb/Kconfig | 6 +
drivers/net/can/usb/Makefile | 1 +
drivers/net/can/usb/mcba_usb.c | 904 +++++++++++++++++++++
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 25 +-
drivers/net/can/vcan.c | 7 +-
drivers/net/can/vxcan.c | 316 +++++++
include/linux/can/core.h | 4 +-
.../linux/can/dev/peak_canfd.h | 86 +-
include/net/netns/can.h | 9 +
include/uapi/linux/can/vxcan.h | 12 +
net/can/af_can.c | 77 +-
net/can/af_can.h | 9 -
net/can/bcm.c | 90 +-
net/can/gw.c | 72 +-
net/can/proc.c | 141 ++--
24 files changed, 3888 insertions(+), 372 deletions(-)
create mode 100644 drivers/net/can/peak_canfd/Kconfig
create mode 100644 drivers/net/can/peak_canfd/Makefile
create mode 100644 drivers/net/can/peak_canfd/peak_canfd.c
create mode 100644 drivers/net/can/peak_canfd/peak_canfd_user.h
create mode 100644 drivers/net/can/peak_canfd/peak_pciefd_main.c
create mode 100644 drivers/net/can/usb/mcba_usb.c
create mode 100644 drivers/net/can/vxcan.c
rename drivers/net/can/usb/peak_usb/pcan_ucan.h => include/linux/can/dev/peak_canfd.h (73%)
create mode 100644 include/uapi/linux/can/vxcan.h
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 0/8] Fix and complete CAN namespace support
From: Oliver Hartkopp @ 2017-04-25 8:43 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can, davem; +Cc: dev, netdev
In-Reply-To: <8fc17f07-f165-970b-f96a-1d920302fd00@pengutronix.de>
On 04/25/2017 08:48 AM, Marc Kleine-Budde wrote:
> On 04/25/2017 08:19 AM, Oliver Hartkopp wrote:
>> Hello Dave,
>>
>> unfortunately the initial network namespace support by Mario Kicherer
>> (8e8cda6d737d) slipped into net-next without further review and Marc pushed
>> the code without my Acked-by. Due to the fact that this code was in net-next
>> now I spent some nights to fix, clean up, finalize and test the missing pieces
>> for the namespace support for the CAN subsystem in net/can.
>>
>> As Marc is currently *VERY* unresponsive on the mailing list due to his 'real'
>
> ... and holidays :) But I'm back in the office now.
I created a cleaned patch series to provide more detailed and proper
patches. And these patches are based on Dave's net-next as you did not
push your latest changes in mkl/linux-can-next to the public on week ago.
When you manage to get this series integrated today then it would be ok
for me. Otherwise we should Dave take this series directly.
Thanks,
Oliver
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] virtio-net: transmit napi
From: Jason Wang @ 2017-04-25 8:39 UTC (permalink / raw)
To: Michael S. Tsirkin, Willem de Bruijn
Cc: Network Development, Willem de Bruijn, David Miller,
virtualization
In-Reply-To: <20170424194015-mutt-send-email-mst@kernel.org>
On 2017年04月25日 00:40, Michael S. Tsirkin wrote:
> On Fri, Apr 21, 2017 at 10:50:12AM -0400, Willem de Bruijn wrote:
>>>>> Maybe I was wrong, but according to Michael's comment it looks like he
>>>>> want
>>>>> check affinity_hint_set just for speculative tx polling on rx napi
>>>>> instead
>>>>> of disabling it at all.
>>>>>
>>>>> And I'm not convinced this is really needed, driver only provide affinity
>>>>> hint instead of affinity, so it's not guaranteed that tx and rx interrupt
>>>>> are in the same vcpus.
>>>> You're right. I made the restriction broader than the request, to really
>>>> err
>>>> on the side of caution for the initial merge of napi tx. And enabling
>>>> the optimization is always a win over keeping it off, even without irq
>>>> affinity.
>>>>
>>>> The cycle cost is significant without affinity regardless of whether the
>>>> optimization is used.
>>>
>>> Yes, I noticed this in the past too.
>>>
>>>> Though this is not limited to napi-tx, it is more
>>>> pronounced in that mode than without napi.
>>>>
>>>> 1x TCP_RR for affinity configuration {process, rx_irq, tx_irq}:
>>>>
>>>> upstream:
>>>>
>>>> 1,1,1: 28985 Mbps, 278 Gcyc
>>>> 1,0,2: 30067 Mbps, 402 Gcyc
>>>>
>>>> napi tx:
>>>>
>>>> 1,1,1: 34492 Mbps, 269 Gcyc
>>>> 1,0,2: 36527 Mbps, 537 Gcyc (!)
>>>> 1,0,1: 36269 Mbps, 394 Gcyc
>>>> 1,0,0: 34674 Mbps, 402 Gcyc
>>>>
>>>> This is a particularly strong example. It is also representative
>>>> of most RR tests. It is less pronounced in other streaming tests.
>>>> 10x TCP_RR, for instance:
>>>>
>>>> upstream:
>>>>
>>>> 1,1,1: 42267 Mbps, 301 Gcyc
>>>> 1,0,2: 40663 Mbps, 445 Gcyc
>>>>
>>>> napi tx:
>>>>
>>>> 1,1,1: 42420 Mbps, 303 Gcyc
>>>> 1,0,2: 42267 Mbps, 431 Gcyc
>>>>
>>>> These numbers were obtained with the virtqueue_enable_cb_delayed
>>>> optimization after xmit_skb, btw. It turns out that moving that before
>>>> increases 1x TCP_RR further to ~39 Gbps, at the cost of reducing
>>>> 100x TCP_RR a bit.
>>>
>>> I see, so I think we can leave the affinity hint optimization/check for
>>> future investigation:
>>>
>>> - to avoid endless optimization (e.g we may want to share a single
>>> vector/napi for tx/rx queue pairs in the future) for this series.
>>> - tx napi is disabled by default which means we can do optimization on top.
>> Okay. I'll drop the vi->affinity_hint_set from the patch set for now.
> I kind of like it, let's be conservative. But I'd prefer a comment
> near it explaining why it's there.
>
Another issue for affinity_hint_set is that it could be changed when
setting channels. I think we've already conservative enough (e.g it was
disabled by default).
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next v3 2/5] virtio-net: transmit napi
From: Jason Wang @ 2017-04-25 8:36 UTC (permalink / raw)
To: Willem de Bruijn, netdev; +Cc: mst, virtualization, davem, Willem de Bruijn
In-Reply-To: <20170424174930.82623-3-willemdebruijn.kernel@gmail.com>
On 2017年04月25日 01:49, Willem de Bruijn wrote:
> @@ -1371,8 +1419,10 @@ static int virtnet_close(struct net_device *dev)
> /* Make sure refill_work doesn't re-enable napi! */
> cancel_delayed_work_sync(&vi->refill);
>
> - for (i = 0; i < vi->max_queue_pairs; i++)
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> napi_disable(&vi->rq[i].napi);
> + napi_disable(&vi->sq[i].napi);
> + }
Looks like this will wait for ever if napi_tx is false because we never
enable the NAPI so we will wait for NAPI_STATE_SCHED to be cleared.
Thanks
^ permalink raw reply
* Network cooling device and how to control NIC speed on thermal condition
From: Waldemar Rymarkiewicz @ 2017-04-25 8:36 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
Hi,
I am not much aware of linux networking architecture so I'd like to
ask first before will start to dig into the code. Appreciate any
feedback.
I am looking on Linux thermal framework and on how to cool down the
system effectively when it hits thermal condition. Already existing
cooling methods cpu_cooling and clock_cooling are good. However, I
wanted to go further and dynamically control also a switch ports'
speed based on thermal condition. Lowering speed means less power,
less power means lower temp.
Is there any in-kernel interface to configure switch port/NIC from other driver?
Is there any mechanism to power save, when port/interface is not
really used (not much or low data traffic), embedded in networking
stack or is it a task for NIC driver itself ?
I was thinking to create net_cooling device similarly to cpu_cooling
device which cool down the system scaling down cpu freq. net_cooling
could lower down interface speed (or tune more parameters to achieve
). Do you thing could this work form networking stack perspective?
Any pointers to the code or a doc highly appreciated.
Thanks,
/Waldek
^ permalink raw reply
* Re: Blogpost evaluation this [PATCH v4 net-next RFC] net: Generic XDP
From: Jesper Dangaard Brouer @ 2017-04-25 8:28 UTC (permalink / raw)
To: David Miller; +Cc: xdp-newbies, netdev, andy, brouer
In-Reply-To: <20170424.182643.485613135674690555.davem@davemloft.net>
On Mon, 24 Apr 2017 18:26:43 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
> Date: Mon, 24 Apr 2017 16:24:05 +0200
>
> > I've done a very detailed evaluation of this patch, and I've created a
> > blogpost like report here:
> >
> > https://prototype-kernel.readthedocs.io/en/latest/blogposts/xdp25_eval_generic_xdp_tx.html
>
> Thanks for doing this Jesper.
>
> > I didn't evaluate the adjust_head part, so I hope Andy is still
> > planning to validate that part?
>
> I was hoping he would post some results today as well.
>
> Andy, how goes it? :)
>
> Once the basic patch is ready and integrated in we can try to do
> xmit_more in generic XDP and see what that does for XDP_TX
> performance.
I agree, we can do xmit_more for generic-XDP later, and it should not
be that hard... basically replacing netdev_start_xmit() with
dev_hard_start_xmit() in generic_xdp_tx(), and finding some place to
store a XDP-skb-pointer (functioning as the skb-list) that will be
"flushed" like __kfree_skb_flush().
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net v2 1/3] net: hns: support deferred probe when can not obtain irq
From: lipeng (Y) @ 2017-04-25 8:21 UTC (permalink / raw)
To: Matthias Brugger, Yankejian, davem, salil.mehta, yisen.zhuang,
huangdaode, zhouhuiru
Cc: netdev, charles.chenxin, linuxarm
In-Reply-To: <377d3197-9b88-b95a-ae79-4348d1448f92@gmail.com>
On 2017/4/24 22:47, Matthias Brugger wrote:
>
>
> On 24/04/17 13:43, lipeng (Y) wrote:
>>
>>
>> On 2017/4/24 18:28, Matthias Brugger wrote:
>>> On 21/04/17 09:44, Yankejian wrote:
>>>> From: lipeng <lipeng321@huawei.com>
>>>>
>>>> In the hip06 and hip07 SoCs, the interrupt lines from the
>>>> DSAF controllers are connected to mbigen hw module.
>>>> The mbigen module is probed with module_init, and, as such,
>>>> is not guaranteed to probe before the HNS driver. So we need
>>>> to support deferred probe.
>>>>
>>>> We check for probe deferral in the hw layer probe, so we not
>>>> probe into the main layer and memories, etc., to later learn
>>>> that we need to defer the probe.
>>>>
>>>
>>> Why? This looks like a hack.
>>> From what I see, we can handle EPROBE_DEFER easily inside hns_ppe_init
>>> checking the return value of hns_rcb_get_cfg. Like you do in 2/3 of
>>> this series.
>>>
>>> Regards,
>>> Matthias
>> Hi Matthias,
>>
>> mdio && phy is not necessary condition, and port can work well for port
>> + SFP (without mdio &&phy).
>>
>> BUT irq is the necessary condition, port can not work well without irq.
>>
>> So, I check IRQ first,and do not probe dsaf if can't obtain irq(1/3 of
>> this series), and check mdio only when there is phy(2/3 of this
>> series).
>>
>> And thanks for your review.
>
> I think I didn't explained myself good enough.
> I was suggesting the following (not even compile tested):
>
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
> index eba406bea52f..be38d47bc399 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
> @@ -510,7 +510,9 @@ int hns_ppe_init(struct dsaf_device *dsaf_dev)
>
> hns_ppe_get_cfg(dsaf_dev->ppe_common[i]);
>
> - hns_rcb_get_cfg(dsaf_dev->rcb_common[i]);
> + ret = hns_rcb_get_cfg(dsaf_dev->rcb_common[i]);
> + if (reg < 0)
> + goto get_cfg_fail;
> }
>
> for (i = 0; i < HNS_PPE_COM_NUM; i++)
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
> index c20a0f4f8f02..c7e801d0c3b7 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
> @@ -492,7 +492,7 @@ static int hns_rcb_get_base_irq_idx(struct
> rcb_common_cb *rcb_common)
> *hns_rcb_get_cfg - get rcb config
> *@rcb_common: rcb common device
> */
> -void hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
> +int hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
> {
> struct ring_pair_cb *ring_pair_cb;
> u32 i;
> @@ -517,10 +517,18 @@ void hns_rcb_get_cfg(struct rcb_common_cb
> *rcb_common)
> ring_pair_cb->virq[HNS_RCB_IRQ_IDX_RX] =
> is_ver1 ? platform_get_irq(pdev, base_irq_idx + i * 2
> + 1) :
> platform_get_irq(pdev, base_irq_idx + i * 3);
> +
> + if ((ring_pair_cb->virq[HNS_RCB_IRQ_IDX_TX] ==
> -EPROBE_DEFER) ||
> + (ring_pair_cb->virq[HNS_RCB_IRQ_IDX_RX] ==
> -EPROBE_DEFER)) {
> + return -EPROBE_DEFER;
> + }
> +
> ring_pair_cb->q.phy_base =
>
> RCB_COMM_BASE_TO_RING_BASE(rcb_common->phy_base, i);
> hns_rcb_ring_pair_get_cfg(ring_pair_cb);
> }
> +
> + return 0;
> }
>
> /**
>
>
> Regards,
> Matthias
Thanks, I will take your advice and test it.
>
>>
>> lipeng
>>
>>>
>>>> Signed-off-by: lipeng <lipeng321@huawei.com>
>>>> Reviewed-by: Yisen Zhuang <yisen.zhuang@huawei.com>
>>>> ---
>>>> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 12 ++++++++++++
>>>> 1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>>>> b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>>>> index 403ea9d..2da5b42 100644
>>>> --- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>>>> +++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
>>>> @@ -2971,6 +2971,18 @@ static int hns_dsaf_probe(struct
>>>> platform_device *pdev)
>>>> struct dsaf_device *dsaf_dev;
>>>> int ret;
>>>>
>>>> + /*
>>>> + * Check if we should defer the probe before we probe the
>>>> + * dsaf, as it's hard to defer later on.
>>>> + */
>>>> + ret = platform_get_irq(pdev, 0);
>>>> + if (ret < 0) {
>>>> + if (ret != -EPROBE_DEFER)
>>>> + dev_err(&pdev->dev, "Cannot obtain irq\n");
>>>> +
>>>> + return ret;
>>>> + }
>>>> +
>>>> dsaf_dev = hns_dsaf_alloc_dev(&pdev->dev, sizeof(struct
>>>> dsaf_drv_priv));
>>>> if (IS_ERR(dsaf_dev)) {
>>>> ret = PTR_ERR(dsaf_dev);
>>>>
>>>
>>> .
>>>
>>
> .
>
^ permalink raw reply
* Re: [RFC 1/4] netlink: make extended ACK setting NULL-friendly
From: Johannes Berg @ 2017-04-25 8:13 UTC (permalink / raw)
To: Jakub Kicinski, netdev
Cc: davem, dsa, daniel, alexei.starovoitov, bblanco, john.fastabend,
kubakici, oss-drivers
In-Reply-To: <20170425080644.122536-2-jakub.kicinski@netronome.com>
On Tue, 2017-04-25 at 01:06 -0700, Jakub Kicinski wrote:
> +#define NL_SET_ERR_MSG(extack, msg) do { \
> + struct netlink_ext_ack *_extack = (extack); \
> + static const char _msg[] = (msg); \
> + \
> + if (_extack) \
> + _extack->_msg = _msg; \
> + else \
> + pr_info("%s\n", _msg); \
> } while (0)
That's a good point, I used it only for genetlink so far where it was
guaranteed non-NULL.
I'm not really sure about the printing though - I'd rather not people
start relying on that and then we convert to have non-NULL and the
message disappears as a result ...
johannes
^ permalink raw reply
* [RFC 4/4] virtio_net: make use of extended ack message reporting
From: Jakub Kicinski @ 2017-04-25 8:06 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, Jakub Kicinski
In-Reply-To: <20170425080644.122536-1-jakub.kicinski@netronome.com>
Try to carry error messages to the user via the netlink extended
ack message attribute.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/virtio_net.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 666ada6130ab..96c5bb31f0af 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1788,7 +1788,8 @@ static int virtnet_reset(struct virtnet_info *vi, int curr_qp, int xdp_qp)
return ret;
}
-static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
+static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack)
{
unsigned long int max_sz = PAGE_SIZE - sizeof(struct padded_vnet_hdr);
struct virtnet_info *vi = netdev_priv(dev);
@@ -1800,16 +1801,17 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO)) {
- netdev_warn(dev, "can't set XDP while host is implementing LRO, disable LRO first\n");
+ NL_SET_ERR_MSG(extack, "can't set XDP while host is implementing LRO, disable LRO first");
return -EOPNOTSUPP;
}
if (vi->mergeable_rx_bufs && !vi->any_header_sg) {
- netdev_warn(dev, "XDP expects header/data in single page, any_header_sg required\n");
+ NL_SET_ERR_MSG(extack, "XDP expects header/data in single page, any_header_sg required");
return -EINVAL;
}
if (dev->mtu > max_sz) {
+ NL_SET_ERR_MSG(extack, "MTU too large to enable XDP");
netdev_warn(dev, "XDP requires MTU less than %lu\n", max_sz);
return -EINVAL;
}
@@ -1820,6 +1822,7 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
/* XDP requires extra queues for XDP_TX */
if (curr_qp + xdp_qp > vi->max_queue_pairs) {
+ NL_SET_ERR_MSG(extack, "Too few free TX rings available");
netdev_warn(dev, "request %i queues but max is %i\n",
curr_qp + xdp_qp, vi->max_queue_pairs);
return -ENOMEM;
@@ -1881,7 +1884,7 @@ static int virtnet_xdp(struct net_device *dev, struct netdev_xdp *xdp)
{
switch (xdp->command) {
case XDP_SETUP_PROG:
- return virtnet_xdp_set(dev, xdp->prog);
+ return virtnet_xdp_set(dev, xdp->prog, xdp->extack);
case XDP_QUERY_PROG:
xdp->prog_attached = virtnet_xdp_query(dev);
return 0;
--
2.11.0
^ permalink raw reply related
* [RFC 3/4] nfp: make use of extended ack message reporting
From: Jakub Kicinski @ 2017-04-25 8:06 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, Jakub Kicinski
In-Reply-To: <20170425080644.122536-1-jakub.kicinski@netronome.com>
Try to carry error messages to the user via the netlink extended
ack message attribute.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net.h | 3 ++-
.../net/ethernet/netronome/nfp/nfp_net_common.c | 22 +++++++++++++---------
.../net/ethernet/netronome/nfp/nfp_net_ethtool.c | 4 ++--
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 8f20fdef0754..48b4a2742233 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -816,7 +816,8 @@ nfp_net_irqs_assign(struct nfp_net *nn, struct msix_entry *irq_entries,
unsigned int n);
struct nfp_net_dp *nfp_net_clone_dp(struct nfp_net *nn);
-int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *new);
+int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *new,
+ struct netlink_ext_ack *extack);
bool nfp_net_link_changed_read_clear(struct nfp_net *nn);
int nfp_net_refresh_eth_port(struct nfp_net *nn);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 8a9b74305493..ff66898375cc 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -2496,24 +2496,27 @@ struct nfp_net_dp *nfp_net_clone_dp(struct nfp_net *nn)
return new;
}
-static int nfp_net_check_config(struct nfp_net *nn, struct nfp_net_dp *dp)
+static int
+nfp_net_check_config(struct nfp_net *nn, struct nfp_net_dp *dp,
+ struct netlink_ext_ack *extack)
{
/* XDP-enabled tests */
if (!dp->xdp_prog)
return 0;
if (dp->fl_bufsz > PAGE_SIZE) {
- nn_warn(nn, "MTU too large w/ XDP enabled\n");
+ NL_SET_ERR_MSG(extack, "MTU too large w/ XDP enabled");
return -EINVAL;
}
if (dp->num_tx_rings > nn->max_tx_rings) {
- nn_warn(nn, "Insufficient number of TX rings w/ XDP enabled\n");
+ NL_SET_ERR_MSG(extack, "Insufficient number of TX rings w/ XDP enabled");
return -EINVAL;
}
return 0;
}
-int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *dp)
+int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *dp,
+ struct netlink_ext_ack *extack)
{
int r, err;
@@ -2525,7 +2528,7 @@ int nfp_net_ring_reconfig(struct nfp_net *nn, struct nfp_net_dp *dp)
dp->num_r_vecs = max(dp->num_rx_rings, dp->num_stack_tx_rings);
- err = nfp_net_check_config(nn, dp);
+ err = nfp_net_check_config(nn, dp, extack);
if (err)
goto exit_free_dp;
@@ -2600,7 +2603,7 @@ static int nfp_net_change_mtu(struct net_device *netdev, int new_mtu)
dp->mtu = new_mtu;
- return nfp_net_ring_reconfig(nn, dp);
+ return nfp_net_ring_reconfig(nn, dp, NULL);
}
static void nfp_net_stat64(struct net_device *netdev,
@@ -2916,9 +2919,10 @@ static int nfp_net_xdp_offload(struct nfp_net *nn, struct bpf_prog *prog)
return ret;
}
-static int nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog)
+static int nfp_net_xdp_setup(struct nfp_net *nn, struct netdev_xdp *xdp)
{
struct bpf_prog *old_prog = nn->dp.xdp_prog;
+ struct bpf_prog *prog = xdp->prog;
struct nfp_net_dp *dp;
int err;
@@ -2945,7 +2949,7 @@ static int nfp_net_xdp_setup(struct nfp_net *nn, struct bpf_prog *prog)
dp->rx_dma_off = 0;
/* We need RX reconfig to remap the buffers (BIDIR vs FROM_DEV) */
- err = nfp_net_ring_reconfig(nn, dp);
+ err = nfp_net_ring_reconfig(nn, dp, xdp->extack);
if (err)
return err;
@@ -2963,7 +2967,7 @@ static int nfp_net_xdp(struct net_device *netdev, struct netdev_xdp *xdp)
switch (xdp->command) {
case XDP_SETUP_PROG:
- return nfp_net_xdp_setup(nn, xdp->prog);
+ return nfp_net_xdp_setup(nn, xdp);
case XDP_QUERY_PROG:
xdp->prog_attached = !!nn->dp.xdp_prog;
return 0;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 6e27d1281425..4d41639b9b03 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -309,7 +309,7 @@ static int nfp_net_set_ring_size(struct nfp_net *nn, u32 rxd_cnt, u32 txd_cnt)
dp->rxd_cnt = rxd_cnt;
dp->txd_cnt = txd_cnt;
- return nfp_net_ring_reconfig(nn, dp);
+ return nfp_net_ring_reconfig(nn, dp, NULL);
}
static int nfp_net_set_ringparam(struct net_device *netdev,
@@ -880,7 +880,7 @@ static int nfp_net_set_num_rings(struct nfp_net *nn, unsigned int total_rx,
if (dp->xdp_prog)
dp->num_tx_rings += total_rx;
- return nfp_net_ring_reconfig(nn, dp);
+ return nfp_net_ring_reconfig(nn, dp, NULL);
}
static int nfp_net_set_channels(struct net_device *netdev,
--
2.11.0
^ permalink raw reply related
* [RFC 2/4] xdp: propagate extended ack to XDP setup
From: Jakub Kicinski @ 2017-04-25 8:06 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, Jakub Kicinski
In-Reply-To: <20170425080644.122536-1-jakub.kicinski@netronome.com>
Drivers usually have a number of restrictions for running XDP
- most common being buffer sizes, LRO and number of rings.
Even though some drivers try to be helpful and print error
messages experience shows that users don't often consult
kernel logs on netlink errors. Try to use the new extended
ack mechanism to carry the message back to user space.
For now the extack is only set for XDP_SETUP_PROG, adding it
to dump/XDP_QUERY_PROG didn't make much sense.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/linux/netdevice.h | 10 ++++++++--
net/core/dev.c | 5 ++++-
net/core/rtnetlink.c | 13 ++++++++-----
3 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5d5267febd56..41667f4238d2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -813,11 +813,16 @@ enum xdp_netdev_command {
XDP_QUERY_PROG,
};
+struct netlink_ext_ack;
+
struct netdev_xdp {
enum xdp_netdev_command command;
union {
/* XDP_SETUP_PROG */
- struct bpf_prog *prog;
+ struct {
+ struct bpf_prog *prog;
+ struct netlink_ext_ack *extack;
+ };
/* XDP_QUERY_PROG */
bool prog_attached;
};
@@ -3283,7 +3288,8 @@ int dev_get_phys_port_id(struct net_device *dev,
int dev_get_phys_port_name(struct net_device *dev,
char *name, size_t len);
int dev_change_proto_down(struct net_device *dev, bool proto_down);
-int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags);
+int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
+ int fd, u32 flags);
struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
diff --git a/net/core/dev.c b/net/core/dev.c
index db6e31564d06..ca4633af5448 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6716,12 +6716,14 @@ EXPORT_SYMBOL(dev_change_proto_down);
/**
* dev_change_xdp_fd - set or clear a bpf program for a device rx path
* @dev: device
+ * @extact: netlink extended ack
* @fd: new program fd or negative value to clear
* @flags: xdp-related flags
*
* Set or clear a bpf program for a device
*/
-int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags)
+int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
+ int fd, u32 flags)
{
const struct net_device_ops *ops = dev->netdev_ops;
struct bpf_prog *prog = NULL;
@@ -6751,6 +6753,7 @@ int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags)
memset(&xdp, 0, sizeof(xdp));
xdp.command = XDP_SETUP_PROG;
+ xdp.extack = extack;
xdp.prog = prog;
err = ops->ndo_xdp(dev, &xdp);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 088f9c8b4196..1723dbb9e3dd 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1909,6 +1909,7 @@ static int do_set_master(struct net_device *dev, int ifindex)
#define DO_SETLINK_NOTIFY 0x03
static int do_setlink(const struct sk_buff *skb,
struct net_device *dev, struct ifinfomsg *ifm,
+ struct netlink_ext_ack *extack,
struct nlattr **tb, char *ifname, int status)
{
const struct net_device_ops *ops = dev->netdev_ops;
@@ -2191,7 +2192,7 @@ static int do_setlink(const struct sk_buff *skb,
}
if (xdp[IFLA_XDP_FD]) {
- err = dev_change_xdp_fd(dev,
+ err = dev_change_xdp_fd(dev, extack,
nla_get_s32(xdp[IFLA_XDP_FD]),
xdp_flags);
if (err)
@@ -2251,7 +2252,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err < 0)
goto errout;
- err = do_setlink(skb, dev, ifm, tb, ifname, 0);
+ err = do_setlink(skb, dev, ifm, extack, tb, ifname, 0);
errout:
return err;
}
@@ -2413,6 +2414,7 @@ EXPORT_SYMBOL(rtnl_create_link);
static int rtnl_group_changelink(const struct sk_buff *skb,
struct net *net, int group,
struct ifinfomsg *ifm,
+ struct netlink_ext_ack *extack,
struct nlattr **tb)
{
struct net_device *dev, *aux;
@@ -2420,7 +2422,7 @@ static int rtnl_group_changelink(const struct sk_buff *skb,
for_each_netdev_safe(net, dev, aux) {
if (dev->group == group) {
- err = do_setlink(skb, dev, ifm, tb, NULL, 0);
+ err = do_setlink(skb, dev, ifm, extack, tb, NULL, 0);
if (err < 0)
return err;
}
@@ -2566,14 +2568,15 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
status |= DO_SETLINK_NOTIFY;
}
- return do_setlink(skb, dev, ifm, tb, ifname, status);
+ return do_setlink(skb, dev, ifm, extack, tb, ifname,
+ status);
}
if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
if (ifm->ifi_index == 0 && tb[IFLA_GROUP])
return rtnl_group_changelink(skb, net,
nla_get_u32(tb[IFLA_GROUP]),
- ifm, tb);
+ ifm, extack, tb);
return -ENODEV;
}
--
2.11.0
^ permalink raw reply related
* [RFC 1/4] netlink: make extended ACK setting NULL-friendly
From: Jakub Kicinski @ 2017-04-25 8:06 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, Jakub Kicinski
In-Reply-To: <20170425080644.122536-1-jakub.kicinski@netronome.com>
As we propagate extended ack reporting throughout various paths in
the kernel it may happen that the same function is called with the
extended ack parameter passed as NULL. Make the NL_SET_ERR_MSG()
macro simply print the message to the logs if that happens.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/linux/netlink.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 8d2a8924705c..b59cfbf2e2c7 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -86,10 +86,14 @@ struct netlink_ext_ack {
* Currently string formatting is not supported (due
* to the lack of an output buffer.)
*/
-#define NL_SET_ERR_MSG(extack, msg) do { \
- static const char _msg[] = (msg); \
- \
- (extack)->_msg = _msg; \
+#define NL_SET_ERR_MSG(extack, msg) do { \
+ struct netlink_ext_ack *_extack = (extack); \
+ static const char _msg[] = (msg); \
+ \
+ if (_extack) \
+ _extack->_msg = _msg; \
+ else \
+ pr_info("%s\n", _msg); \
} while (0)
extern void netlink_kernel_release(struct sock *sk);
--
2.11.0
^ permalink raw reply related
* [RFC 0/4] xdp: use netlink extended ACK reporting
From: Jakub Kicinski @ 2017-04-25 8:06 UTC (permalink / raw)
To: netdev
Cc: davem, johannes, dsa, daniel, alexei.starovoitov, bblanco,
john.fastabend, kubakici, oss-drivers, Jakub Kicinski
Hi!
This series is an attempt to make XDP more user friendly by
enabling exploiting the recently added netlink extended ACK
reporting to carry messages to user space.
I made iproute2 parse the extended messages and have it showing
the errors like this:
# ip link set dev p4p1 xdp obj ipip_prepend.o sec ".text"
RTNETLINK answers: Invalid argument (MTU too large w/ XDP enabled)
Where the message is coming directly from the driver. There could
still be a bit of a leap for a complete novice from the message
above to the right settings. I wonder if it would be worthwhile
adding #defines for the most common configuration conflicts?
Sharing the messages verbatim between drivers could make them easier
to google.
Also - is anyone working on adding proper extack support to iproute2?
The code I have right now is a bit of a hack...
Jakub Kicinski (4):
netlink: make extended ACK setting NULL-friendly
xdp: propagate extended ack to XDP setup
nfp: make use of extended ack message reporting
virtio_net: make use of extended ack message reporting
drivers/net/ethernet/netronome/nfp/nfp_net.h | 3 ++-
.../net/ethernet/netronome/nfp/nfp_net_common.c | 22 +++++++++++++---------
.../net/ethernet/netronome/nfp/nfp_net_ethtool.c | 4 ++--
drivers/net/virtio_net.c | 11 +++++++----
include/linux/netdevice.h | 10 ++++++++--
include/linux/netlink.h | 12 ++++++++----
net/core/dev.c | 5 ++++-
net/core/rtnetlink.c | 13 ++++++++-----
8 files changed, 52 insertions(+), 28 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: Ensure pointer correctly set if skb data location changes
From: Arend Van Spriel @ 2017-04-25 7:56 UTC (permalink / raw)
To: Kalle Valo
Cc: James Hughes, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170424114050.24948-1-james.hughes-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
On 24-4-2017 13:40, James Hughes wrote:
> The incoming skb header may be resized if header space is
> insufficient, which might change the data adddress in the skb.
> Ensure that a cached pointer to that data is correctly set by
> moving assignment to after any possible changes.
Hi Kalle,
This one should go on 4.12 queue as well.
Thanks,
Arend
> Signed-off-by: James Hughes <james.hughes-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
>
> Acked-by: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
^ permalink raw reply
* Re: qed*: debug infrastructures
From: Jiri Pirko @ 2017-04-25 7:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Elior, Ariel, David Miller, netdev@vger.kernel.org, Mintz, Yuval,
Tayar, Tomer, Dupuis, Chad
In-Reply-To: <20170424204410.538fdabc@cakuba.netronome.com>
Tue, Apr 25, 2017 at 05:44:10AM CEST, kubakici@wp.pl wrote:
>On Mon, 24 Apr 2017 17:38:57 +0000, Elior, Ariel wrote:
>> Hi Dave,
>
>Hi Ariel!
>
>I'm not Dave but let me share my perspective :)
>
>> According to the recent messages on the list indicating debugfs is not the way
>> to go, I am looking for some guidance on what is. dpipe approach was
>> mentioned as favorable, but I wanted to make sure molding our debug features to
>> this infrastructure will result in something acceptable. A few points:
>>
>> 1.
>> One of our HW debug features is a signal recording feature. HW is configured to
>> output specific signals, which are then continuously dumped into a cyclic
>> buffer on host. There are ~8000 signals, which can be logically divided to ~100
>> groups. I believe this can be modeled in dpipe (or similar tool) as a set of
>> ~100 tables with ~80 entries each, and user would be able to see them all and
>> choose what they like. The output data itself is binary, and meaningless to
>> "the user". The amount of data is basically as large a contiguous buffer as
>> driver can allocate, i.e. usually 4MB. When user selects the signals, and sets
>> meta data regarding to mode of operations, some device configuration will have
>> to take place. Does that sound reasonable?
>> This debug feature already exists out of tree for bnx2x and qed* drivers and is
>> *very* effective in field deployments. I would very much like to see this as an
>> in-tree feature via some infrastructure or another.
>
>Sorry for even mentioning it, new debug interfaces would be cool, but
>for FW/HW state dumps which are meaningless to the user why not just
>use ethtool get-dump/set-dump? Do you really need the ability to
>toggle those 8k signals one-by-one or are there reasonable sets you
>could provide from the driver that you could encode on the available
>32bits of flags?
>
>What could be useful would be some form of start/stop commands for
>debugging to tell the driver/FW when to record events selected by
>set-dump and maybe a way for the user to discover what dumps the driver
>can provide (a'la ethtool private flags).
>
>> 2.
>> Sometimes we want to debug the probe or removal flow of the driver. ethtool has
>> the disadvantage of only being available once network device is available. If a
>> bug stops the load flow before the ethtool debug paths are available, there is
>> no way to collect a dump. Similarly, removal flows which hit a bug but do remove
>> the network device, can't be debugged from ethtool. Does dpipe suffer from the
>> same problem? qed* (like mlx*) has a common-functionality module. This allows
>> creating debugfs nodes even before the network drivers are probed, providing a
>> solution for this (debug nodes are also available after network driver removal).
>> If dpipe does hold an answer here (e.g. provide preconfiguration which would
>> kick in when network device registers) then we might want to port all of our
>> register dump logic over there for this advantage. Does that sound reasonable?
>
>Porting the debug/dump infrastructure to devlink would be very much
>appreciated. I'm not sure it would fit into dpipe or be a separate
>command.
Yeah. dpipe was designed to provide HW pipeline abstraction, based on
match/action model. I think that for stats and debugging, it would make
sense to introduce another devlink object.
>
>> 3.
>> Yuval mentioned this, but I wanted to reiterate that the same is necessary for
>> our storage drivers (qedi/qedf). debugfs does have the advantage of being non
>> sub-system specific. Is there perhaps another non subsystem specific debug
>> infrastructure which *is* acceptable to the networking subsystem? My guess is
>> that the storage drivers will turn to debugfs (in their own subsystem).
>
>devlink is not ethernet-specific, it should be a good fit for iSCSI and
>FCOE drivers, too.
Yes. During the devlink design, I had the non-network usecase in mind.
Devlink is the iface to be used for all who need it.
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: Make skb header writable before use
From: Kalle Valo @ 2017-04-25 7:49 UTC (permalink / raw)
To: Arend Van Spriel
Cc: James Hughes, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ddb49ce2-9968-534e-dd17-b1bf21513c5d-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Arend Van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> writes:
> On 25-4-2017 9:38, Arend Van Spriel wrote:
>> On 24-4-2017 15:03, James Hughes wrote:
>>> The driver was making changes to the skb_header without
>>> ensuring it was writable (i.e. uncloned).
>>> This patch also removes some boiler plate header size
>>> checking/adjustment code as that is also handled by the
>>> skb_cow_header function used to make header writable.
>>>
>>> This patch depends on
>>> brcmfmac: Ensure pointer correctly set if skb data location changes
>>>
>>> Signed-off-by: James Hughes <james.hughes-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
>>> ---
>>> Changes in v2
>>> Makes the _cow_ call at the entry point of the skb in to the
>>> stack, means only needs to be done once, and error handling
>>> is easier.
>>> Split a separate minor bug fix off to a separate patch (which
>>> this patch depends on)
>>
>> Hi James,
>>
>> Can you please indicate in this section that you want it applied for
>> 4.12 as it is an important fix. Probably will have to wait until after
>> the merge window before it can get in the wireless-drivers repo.
>
> Hi Kalle,
>
> I should have checked kernel mailing list first as Linus added another
> rc cycle. So can this patch still go in wireless-drivers-next for 4.12
> kernel.
Yup, I'll queue it for 4.12.
> I want it for stable as well, but I will look at that when it lands
> upstream.
Ok, so I don't add any stable tag during commit.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: Make skb header writable before use
From: Arend Van Spriel @ 2017-04-25 7:46 UTC (permalink / raw)
To: Kalle Valo
Cc: James Hughes, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ef75a194-2e0b-6586-7b31-2eb83128d2bc-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
On 25-4-2017 9:38, Arend Van Spriel wrote:
> On 24-4-2017 15:03, James Hughes wrote:
>> The driver was making changes to the skb_header without
>> ensuring it was writable (i.e. uncloned).
>> This patch also removes some boiler plate header size
>> checking/adjustment code as that is also handled by the
>> skb_cow_header function used to make header writable.
>>
>> This patch depends on
>> brcmfmac: Ensure pointer correctly set if skb data location changes
>>
>> Signed-off-by: James Hughes <james.hughes-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
>> ---
>> Changes in v2
>> Makes the _cow_ call at the entry point of the skb in to the
>> stack, means only needs to be done once, and error handling
>> is easier.
>> Split a separate minor bug fix off to a separate patch (which
>> this patch depends on)
>
> Hi James,
>
> Can you please indicate in this section that you want it applied for
> 4.12 as it is an important fix. Probably will have to wait until after
> the merge window before it can get in the wireless-drivers repo.
Hi Kalle,
I should have checked kernel mailing list first as Linus added another
rc cycle. So can this patch still go in wireless-drivers-next for 4.12
kernel. I want it for stable as well, but I will look at that when it
lands upstream.
Regards,
Arend
> Regards,
> Arend
>
>> .../net/wireless/broadcom/brcm80211/brcmfmac/core.c | 19 ++++---------------
>> 1 file changed, 4 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> index 9b7c19a508ac..88f8675a94c2 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> @@ -211,22 +211,11 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
>> goto done;
>> }
>>
>> - /* Make sure there's enough room for any header */
>> - if (skb_headroom(skb) < drvr->hdrlen) {
>> - struct sk_buff *skb2;
>> -
>> - brcmf_dbg(INFO, "%s: insufficient headroom\n",
>> - brcmf_ifname(ifp));
>> - drvr->bus_if->tx_realloc++;
>> - skb2 = skb_realloc_headroom(skb, drvr->hdrlen);
>> + /* Make sure there's enough room for any header, and make it writable */
>> + if (skb_cow_head(skb, drvr->hdrlen)) {
>> dev_kfree_skb(skb);
>> - skb = skb2;
>> - if (skb == NULL) {
>> - brcmf_err("%s: skb_realloc_headroom failed\n",
>> - brcmf_ifname(ifp));
>> - ret = -ENOMEM;
>> - goto done;
>> - }
>> + ret = -ENOMEM;
>> + goto done;
>> }
>>
>> /* validate length for ether packet */
>>
^ permalink raw reply
* [RFC PATCH] netxen_nic: null-terminate serial number string in netxen_check_options()
From: Jerome Marchand @ 2017-04-25 7:42 UTC (permalink / raw)
To: Manish Chopra, Rahul Verma, Dept-GELinuxNICDev; +Cc: netdev, linux-kernel
The serial_num string in netxen_check_options() is not always properly
null-terminated. I couldn't find the documention on the serial number
format and I suspect a proper integer to string conversion is in
order, but this patch a least prevents the out-of-bound access.
It solves the following kasan warning:
[ 36.127074] ==================================================================
[ 36.168472] BUG: KASAN: stack-out-of-bounds in strnlen+0x38/0x60 at addr ffff8800360e7a50
[ 36.216956] Read of size 1 by task kworker/0:1/188
[ 36.244451] page:ffffea0000d839c0 count:0 mapcount:0 mapping: (null) index:0x2
[ 36.291475] page flags: 0x1fffff00000000()
[ 36.314980] page dumped because: kasan: bad access detected
[ 36.348117] CPU: 0 PID: 188 Comm: kworker/0:1 Not tainted 3.10.0-650.el7.test.kasan.x86_64 #1
[ 36.397065] Hardware name: HP ProLiant DL585 G7, BIOS A16 03/19/2012
[ 36.434443] Workqueue: events work_for_cpu_fn
[ 36.459452] ffff8800360e7a30 00000000e4708e04 ffff8800360e7538 ffffffffb37748bf
[ 36.503442] ffff8800360e75c0 ffffffffb2f4a7e7 ffff8800360d8948 0000000600000007
[ 36.546616] ffff8800360d8950 0000000000000086 ffffffffb3782086 0000000000000004
[ 36.589439] Call Trace:
[ 36.603611] [<ffffffffb37748bf>] dump_stack+0x19/0x1b
[ 36.633970] [<ffffffffb2f4a7e7>] kasan_report_error+0x507/0x540
[ 36.668472] [<ffffffffb3782086>] ? _raw_spin_unlock_irqrestore+0x36/0x80
[ 36.708967] [<ffffffffb2f4ae48>] kasan_report+0x58/0x60
[ 36.740311] [<ffffffffb2d5bf00>] ? cpu_clock+0x10/0x20
[ 36.771532] [<ffffffffb3182e68>] ? strnlen+0x38/0x60
[ 36.800430] [<ffffffffb2f48e6d>] __asan_load1+0x4d/0x50
[ 36.831977] [<ffffffffb3182e68>] strnlen+0x38/0x60
[ 36.859995] [<ffffffffb3186e4f>] string.isra.7+0x3f/0x130
[ 36.891531] [<ffffffffb3189b60>] vsnprintf+0x620/0xd70
[ 36.922997] [<ffffffffb2eba659>] ? __free_pages_ok+0xe9/0x160
[ 36.956467] [<ffffffffb3189540>] ? pointer.isra.19+0x780/0x780
[ 36.991095] [<ffffffffb2ce6ecf>] ? vprintk_emit+0x12f/0x730
[ 37.023440] [<ffffffffb318a2bd>] vscnprintf+0xd/0x40
[ 37.053146] [<ffffffffb2ce6efd>] vprintk_emit+0x15d/0x730
[ 37.084983] [<ffffffffc01afea1>] ? netxen_setup_minidump+0x621/0x780 [netxen_nic]
[ 37.129435] [<ffffffffb2ce784e>] vprintk_default+0x3e/0x60
[ 37.161962] [<ffffffffb376f32a>] printk+0xa1/0xc8
[ 37.189446] [<ffffffffb376f289>] ? panic+0x28d/0x28d
[ 37.219447] [<ffffffffc01a0014>] netxen_start_firmware+0x1124/0x1170 [netxen_nic]
[ 37.262989] [<ffffffffc019eef0>] ? netxen_show_diag_mode+0x50/0x50 [netxen_nic]
[ 37.306968] [<ffffffffc019a960>] ? netxen_nic_hw_write_wx_2M+0x180/0x180 [netxen_nic]
[ 37.352621] [<ffffffffc019a9dc>] ? netxen_nic_hw_read_wx_2M+0x7c/0x180 [netxen_nic]
[ 37.397967] [<ffffffffc01a2863>] netxen_nic_probe+0x6f3/0x15f0 [netxen_nic]
[ 37.439351] [<ffffffffb2c5a3c7>] ? native_sched_clock+0xf7/0x190
[ 37.474980] [<ffffffffb2daf726>] ? mark_lock+0xd6/0x860
[ 37.505439] [<ffffffffc01a2170>] ? netxen_nic_open+0xc0/0xc0 [netxen_nic]
[ 37.545988] [<ffffffffb3782086>] ? _raw_spin_unlock_irqrestore+0x36/0x80
[ 37.584974] [<ffffffffb2db01e7>] ? trace_hardirqs_on_caller+0x187/0x2b0
[ 37.625444] [<ffffffffb2db031d>] ? trace_hardirqs_on+0xd/0x10
[ 37.658978] [<ffffffffb37820a9>] ? _raw_spin_unlock_irqrestore+0x59/0x80
[ 37.698937] [<ffffffffc01a2170>] ? netxen_nic_open+0xc0/0xc0 [netxen_nic]
[ 37.738975] [<ffffffffb31edffa>] local_pci_probe+0x7a/0xd0
[ 37.771447] [<ffffffffb2d21d4f>] ? process_one_work+0x36f/0xb80
[ 37.806447] [<ffffffffb31edf80>] ? pci_device_shutdown+0xa0/0xa0
[ 37.841483] [<ffffffffb2d1a3dc>] work_for_cpu_fn+0x2c/0x50
[ 37.873443] [<ffffffffb2d21df6>] process_one_work+0x416/0xb80
[ 37.908116] [<ffffffffb2d21d4f>] ? process_one_work+0x36f/0xb80
[ 37.943456] [<ffffffffb2d219e0>] ? flush_delayed_work+0x80/0x80
[ 37.977968] [<ffffffffb2d1b2d3>] ? move_linked_works+0x83/0xb0
[ 38.013461] [<ffffffffb2d2292c>] worker_thread+0x3cc/0x580
[ 38.045479] [<ffffffffb2d22560>] ? process_one_work+0xb80/0xb80
[ 38.081445] [<ffffffffb2d2fcce>] kthread+0x16e/0x180
[ 38.110450] [<ffffffffb2d2fb60>] ? flush_kthread_work+0x280/0x280
[ 38.145996] [<ffffffffb2c5a589>] ? sched_clock+0x9/0x10
[ 38.177466] [<ffffffffb2d48bc9>] ? finish_task_switch+0x59/0x200
[ 38.212477] [<ffffffffb2d2fb60>] ? flush_kthread_work+0x280/0x280
[ 38.248158] [<ffffffffb3792b98>] ret_from_fork+0x58/0x90
[ 38.279982] [<ffffffffb2d2fb60>] ? flush_kthread_work+0x280/0x280
[ 38.315480] Memory state around the buggy address:
[ 38.344557] ffff8800360e7900: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 04 f4
[ 38.386125] ffff8800360e7980: f4 f4 f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 00
[ 38.428978] >ffff8800360e7a00: 00 00 f2 f2 f2 f2 00 00 00 00 f3 f3 f3 f3 00 00
[ 38.470442] ^
[ 38.505984] ffff8800360e7a80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 38.547465] ffff8800360e7b00: 00 00 00 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2
[ 38.590467] ==================================================================
Signed-off-by: Jerome Marchand <jmarchan@redhat.com>
---
drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 827de83..4d9cefc 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -842,7 +842,7 @@ netxen_check_options(struct netxen_adapter *adapter)
{
u32 fw_major, fw_minor, fw_build, prev_fw_version;
char brd_name[NETXEN_MAX_SHORT_NAME];
- char serial_num[32];
+ char serial_num[33];
int i, offset, val, err;
__le32 *ptr32;
struct pci_dev *pdev = adapter->pdev;
@@ -861,6 +861,7 @@ netxen_check_options(struct netxen_adapter *adapter)
ptr32[i] = cpu_to_le32(val);
offset += sizeof(u32);
}
+ serial_num[32] = 0;
fw_major = NXRD32(adapter, NETXEN_FW_VERSION_MAJOR);
fw_minor = NXRD32(adapter, NETXEN_FW_VERSION_MINOR);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] brcmfmac: Make skb header writable before use
From: Arend Van Spriel @ 2017-04-25 7:38 UTC (permalink / raw)
To: James Hughes, Franky Lin, Hante Meuleman, Kalle Valo,
linux-wireless, brcm80211-dev-list.pdl, netdev
In-Reply-To: <20170424130322.476-1-james.hughes@raspberrypi.org>
On 24-4-2017 15:03, James Hughes wrote:
> The driver was making changes to the skb_header without
> ensuring it was writable (i.e. uncloned).
> This patch also removes some boiler plate header size
> checking/adjustment code as that is also handled by the
> skb_cow_header function used to make header writable.
>
> This patch depends on
> brcmfmac: Ensure pointer correctly set if skb data location changes
>
> Signed-off-by: James Hughes <james.hughes@raspberrypi.org>
> ---
> Changes in v2
> Makes the _cow_ call at the entry point of the skb in to the
> stack, means only needs to be done once, and error handling
> is easier.
> Split a separate minor bug fix off to a separate patch (which
> this patch depends on)
Hi James,
Can you please indicate in this section that you want it applied for
4.12 as it is an important fix. Probably will have to wait until after
the merge window before it can get in the wireless-drivers repo.
Regards,
Arend
> .../net/wireless/broadcom/brcm80211/brcmfmac/core.c | 19 ++++---------------
> 1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 9b7c19a508ac..88f8675a94c2 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -211,22 +211,11 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
> goto done;
> }
>
> - /* Make sure there's enough room for any header */
> - if (skb_headroom(skb) < drvr->hdrlen) {
> - struct sk_buff *skb2;
> -
> - brcmf_dbg(INFO, "%s: insufficient headroom\n",
> - brcmf_ifname(ifp));
> - drvr->bus_if->tx_realloc++;
> - skb2 = skb_realloc_headroom(skb, drvr->hdrlen);
> + /* Make sure there's enough room for any header, and make it writable */
> + if (skb_cow_head(skb, drvr->hdrlen)) {
> dev_kfree_skb(skb);
> - skb = skb2;
> - if (skb == NULL) {
> - brcmf_err("%s: skb_realloc_headroom failed\n",
> - brcmf_ifname(ifp));
> - ret = -ENOMEM;
> - goto done;
> - }
> + ret = -ENOMEM;
> + goto done;
> }
>
> /* validate length for ether packet */
>
^ permalink raw reply
* Re: [PATCH] stmmac: Add support for SIMATIC IOT2000 platform
From: Andy Shevchenko @ 2017-04-25 7:30 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, netdev,
Linux Kernel Mailing List, Sascha Weisenberger
In-Reply-To: <feb4856b-fb66-3861-2121-d2834fb01899@siemens.com>
On Tue, Apr 25, 2017 at 8:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-04-24 23:27, Andy Shevchenko wrote:
>> On Mon, Apr 24, 2017 at 10:27 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> The IOT2000 is industrial controller platform, derived from the Intel
>>> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
>>> IOT2040 has two of them. They can be told apart based on the board asset
>>> tag in the DMI table.
>>> + const char *asset_tag;
>>
>> I guess this is redundant. See below.
>>
>>> + {
>>> + .name = "SIMATIC IOT2000",
>>> + .asset_tag = "6ES7647-0AA00-0YA2",
>>> + .func = 6,
>>> + .phy_addr = 1,
>>> + },
>>
>> The below has same definition disregard on asset_tag.
>>
>
> There is a small difference in the asset tag, just not at the last digit
> where one may expect it, look:
>
> ...-0YA2 -> IOT2020
> ...-1YA2 -> IOT2040
Yes. And how does it change my statement? You may use one record here
instead of two.
>
>>> + {
>>> + .name = "SIMATIC IOT2000",
>>> + .asset_tag = "6ES7647-0AA00-1YA2",
>>> + .func = 6,
>>> + .phy_addr = 1,
>>> + },
>>> + {
>>> + .name = "SIMATIC IOT2000",
>>> + .asset_tag = "6ES7647-0AA00-1YA2",
>>> + .func = 7,
>>> + .phy_addr = 1,
>>> + },
>>
>> How this supposed to work if phy_addr is the same?
> That address space is MAC-local, and we have two different MACs here.
Got it, though asset_tag here is redundant as well.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized stats
From: Brown, Aaron F @ 2017-04-25 7:10 UTC (permalink / raw)
To: Benjamin Poirier, Neftin, Sasha
Cc: Kirsher@f1.synalogic.ca, Stefan Priebe, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org
In-Reply-To: <20170424191026.wpnpyhdaurfjixl7@f1.synalogic.ca>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Benjamin Poirier
> Sent: Monday, April 24, 2017 12:10 PM
> To: Neftin, Sasha <sasha.neftin@intel.com>
> Cc: Kirsher@f1.synalogic.ca; Stefan Priebe <s.priebe@profihost.ag>;
> netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized
> stats
>
> Sasha, please use reply-all to keep everyone in cc (including me...).
>
> On 2017/04/24 11:17, Neftin, Sasha wrote:
> > On 4/23/2017 15:53, Neftin, Sasha wrote:
> > > -----Original Message-----
> > > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org]
> On Behalf Of Benjamin Poirier
> > > Sent: Saturday, April 22, 2017 00:20
> > > To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> > > Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Stefan
> Priebe <s.priebe@profihost.ag>
> > > Subject: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized
> stats
> > >
> > > Some statistics passed to ethtool are garbage because
> e1000e_get_stats64() doesn't write them, for example: tx_heartbeat_errors.
> This leaks kernel memory to userspace and confuses users.
> > >
> > > Do like ixgbe and use dev_get_stats() which first zeroes out
> rtnl_link_stats64.
> > >
> > > Reported-by: Stefan Priebe <s.priebe@profihost.ag>
> > > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > > ---
> > > drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c
> b/drivers/net/ethernet/intel/e1000e/ethtool.c
> > > index 7aff68a4a4df..f117b90cdc2f 100644
> > > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> > > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> > > @@ -2063,7 +2063,7 @@ static void e1000_get_ethtool_stats(struct
> net_device *netdev,
> > > pm_runtime_get_sync(netdev->dev.parent);
> > > - e1000e_get_stats64(netdev, &net_stats);
> > > + dev_get_stats(netdev, &net_stats);
> > > pm_runtime_put_sync(netdev->dev.parent);
> > > --
> > > 2.12.2
> > >
> > > _______________________________________________
> > > Intel-wired-lan mailing list
> > > Intel-wired-lan@lists.osuosl.org
> > > http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> >
> > Hello,
> >
> > We would like to not accept this patch. Suggested generic method
> > '*dev_get_stats' (net/core/dev.c) calls 'ops->ndo_get_stats64' method
> which
> > eventually calls e1000e_get_stats64 (netdev.c) - so there is same
> > functionality. Also, see that 'e1000e_get_stats64' method in netdev.c (line
>
> No, it's not the same functionality because dev_get_stats() does a
> memset on the rtnl_link_stats64 struct.
>
> > 5928) calls 'memset' with 0's before update statistics. Local sanity check
>
> I don't see any memset in e1000e_get_stats64(). What kernel version are
> you looking at?
The call to memset was removed from the upstream kernel with:
------------------------------------------------------------------------------------
commit 5944701df90d9577658e2354cc27c4ceaeca30fe
Author: stephen hemminger <stephen@networkplumber.org>
Date: Fri Jan 6 19:12:53 2017 -0800
net: remove useless memset's in drivers get_stats64
In dev_get_stats() the statistic structure storage has already been
zeroed. Therefore network drivers do not need to call memset() again.
...
< changes to other drivers snipped out >
...
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/int
index 723025b..79651eb 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5925,7 +5925,6 @@ void e1000e_get_stats64(struct net_device *netdev,
{
struct e1000_adapter *adapter = netdev_priv(netdev);
- memset(stats, 0, sizeof(struct rtnl_link_stats64));
spin_lock(&adapter->stats64_lock);
e1000e_update_stats(adapter);
/* Fill out the OS statistics structure */
------------------------------------------------------------------------------------
This also is where the bad counters start to show up for e1000e for my test systems. From this driver on I see (very) large values for tx_dropped, rx_over_errors and tx_fifo_errors on driver load (even before bringing the interface up. It seems the memset is not so useless for this driver after all. Would simply reverting the e1000e portion of this patch resolve the issue?
>
> > in our lab shows 'tx_heartbeat_errors' counter reported as 0.
> >
>
> Please see the mail I just sent to Paul Menzel <pmenzel@molgen.mpg.de>
> for more information about the issue and how to reproduce it.
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related
* [PATCH net-next] qed: fix invalid use of sizeof in qed_alloc_qm_data()
From: Wei Yongjun @ 2017-04-25 7:07 UTC (permalink / raw)
To: Yuval Mintz, Ariel Elior; +Cc: Wei Yongjun, everest-linux-l2, netdev
From: Wei Yongjun <weiyongjun1@huawei.com>
sizeof() when applied to a pointer typed expression gives the
size of the pointer, not that of the pointed data.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.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 2498785..4fa5a1e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -820,7 +820,7 @@ static int qed_alloc_qm_data(struct qed_hwfn *p_hwfn)
if (!qm_info->qm_vport_params)
goto alloc_err;
- qm_info->qm_port_params = kzalloc(sizeof(qm_info->qm_port_params) *
+ qm_info->qm_port_params = kzalloc(sizeof(*qm_info->qm_port_params) *
p_hwfn->cdev->num_ports_in_engines,
GFP_KERNEL);
if (!qm_info->qm_port_params)
^ permalink raw reply related
* Re: [PATCH 0/8] Fix and complete CAN namespace support
From: Marc Kleine-Budde @ 2017-04-25 6:48 UTC (permalink / raw)
To: Oliver Hartkopp, linux-can, davem; +Cc: dev, netdev
In-Reply-To: <20170425061945.28722-1-socketcan@hartkopp.net>
[-- Attachment #1.1: Type: text/plain, Size: 870 bytes --]
On 04/25/2017 08:19 AM, Oliver Hartkopp wrote:
> Hello Dave,
>
> unfortunately the initial network namespace support by Mario Kicherer
> (8e8cda6d737d) slipped into net-next without further review and Marc pushed
> the code without my Acked-by. Due to the fact that this code was in net-next
> now I spent some nights to fix, clean up, finalize and test the missing pieces
> for the namespace support for the CAN subsystem in net/can.
>
> As Marc is currently *VERY* unresponsive on the mailing list due to his 'real'
... and holidays :) But I'm back in the office now.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
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