* Re: [PATCH] net: moxa: inherit DMA masks to make dma_map_single() work
From: Andrew Lunn @ 2022-08-12 16:13 UTC (permalink / raw)
To: Sergei Antonov
Cc: netdev, Jakub Kicinski, Pavel Skripkin, David S . Miller,
Guobin Huang, Paolo Abeni
In-Reply-To: <20220812154820.2225457-1-saproj@gmail.com>
On Fri, Aug 12, 2022 at 06:48:20PM +0300, Sergei Antonov wrote:
> dma_map_single() calls fail in moxart_mac_setup_desc_ring() and
> moxart_mac_start_xmit() which leads to an incessant output of this:
>
> [ 16.043925] moxart-ethernet 92000000.mac eth0: DMA mapping error
> [ 16.050957] moxart-ethernet 92000000.mac eth0: DMA mapping error
> [ 16.058229] moxart-ethernet 92000000.mac eth0: DMA mapping error
>
> To make dma_map_single() work, inherit DMA masks from the platform device.
>
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> CC: Jakub Kicinski <kuba@kernel.org>
> CC: Pavel Skripkin <paskripkin@gmail.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Guobin Huang <huangguobin4@huawei.com>
> CC: Paolo Abeni <pabeni@redhat.com>
> ---
> drivers/net/ethernet/moxa/moxart_ether.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
> index a3214a762e4b..de99211d85c2 100644
> --- a/drivers/net/ethernet/moxa/moxart_ether.c
> +++ b/drivers/net/ethernet/moxa/moxart_ether.c
> @@ -537,6 +537,10 @@ static int moxart_mac_probe(struct platform_device *pdev)
> ndev->priv_flags |= IFF_UNICAST_FLT;
> ndev->irq = irq;
>
> + /* Inherit the DMA masks from the platform device */
> + ndev->dev.dma_mask = p_dev->dma_mask;
> + ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask;
There is only one other ethernet driver which does this. What you see
much more often is:
alacritech/slicoss.c: paddr = dma_map_single(&sdev->pdev->dev, skb->data, maplen,
neterion/s2io.c: dma_map_single(&sp->pdev->dev, ba->ba_1,
dlink/dl2k.c: cpu_to_le64(dma_map_single(&np->pdev->dev, skb->data,
micrel/ksz884x.c: dma_buf->dma = dma_map_single(&hw_priv->pdev->dev, skb->data,
Andrew
^ permalink raw reply
* RE: [PATCH bpf-next v2] skmsg: Fix wrong last sg check in sk_msg_recvmsg()
From: John Fastabend @ 2022-08-12 15:55 UTC (permalink / raw)
To: Liu Jian, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
daniel, andrii, netdev, bpf
Cc: liujian56
In-Reply-To: <20220809094915.150391-1-liujian56@huawei.com>
Liu Jian wrote:
> Fix one kernel NULL pointer dereference as below:
>
> [ 224.462334] Call Trace:
> [ 224.462394] __tcp_bpf_recvmsg+0xd3/0x380
> [ 224.462441] ? sock_has_perm+0x78/0xa0
> [ 224.462463] tcp_bpf_recvmsg+0x12e/0x220
> [ 224.462494] inet_recvmsg+0x5b/0xd0
> [ 224.462534] __sys_recvfrom+0xc8/0x130
> [ 224.462574] ? syscall_trace_enter+0x1df/0x2e0
> [ 224.462606] ? __do_page_fault+0x2de/0x500
> [ 224.462635] __x64_sys_recvfrom+0x24/0x30
> [ 224.462660] do_syscall_64+0x5d/0x1d0
> [ 224.462709] entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> In commit 9974d37ea75f ("skmsg: Fix invalid last sg check in
> sk_msg_recvmsg()"), we change last sg check to sg_is_last(),
> but in sockmap redirection case (without stream_parser/stream_verdict/
> skb_verdict), we did not mark the end of the scatterlist. Check the
> sk_msg_alloc, sk_msg_page_add, and bpf_msg_push_data functions, they all
> do not mark the end of sg. They are expected to use sg.end for end
> judgment. So the judgment of '(i != msg_rx->sg.end)' is added back here.
>
> Fixes: 9974d37ea75f ("skmsg: Fix invalid last sg check in sk_msg_recvmsg()")
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
OK lets take this. Thanks.
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 net-next] net: ethernet: ti: davinci_mdio: Add workaround for errata i2329
From: Andrew Lunn @ 2022-08-12 15:54 UTC (permalink / raw)
To: Ravi Gunasekaran
Cc: davem, edumazet, kuba, pabeni, linux-omap, netdev, linux-kernel,
linux-arm-kernel, kishon, vigneshr
In-Reply-To: <ed3554bc-af62-78ce-a3eb-ff5f27ade6a2@ti.com>
> sh_eth is not configured for autosuspend and uses only pm_runtime_put().
I don't know the runtime power management code very well. We should
probably ask somebody how does. However:
https://elixir.bootlin.com/linux/latest/source/drivers/base/power/runtime.c#L168
This suggests it should be safe to perform an auto suspend on a device
which is not configured for auto suspend. To me, it looks like is
should directly suspend.
Devices are in a tree. If you suspend a leaf, you can walk up the tree
and suspend its parent as well, if it is not needed. Similarly, if you
wake a leaf, you need its parents awake as well, so you need to walk
up the tree and wake them. In order for this to work reliably, i
expect runtime PM to be very tolerant. If a device is not configured
for runtime PM, actions should be a NOP. If it is not configured for
auto suspend, and you ask it to auto suspend, to me, it would make
sense for it to immediately suspend, etc.
> Please provide your views on this. Your inputs on the next course of action
> would be helpful.
I would suggest you talk to somebody who knows about runtime PM.
Andrew
^ permalink raw reply
* [PATCH] net: moxa: inherit DMA masks to make dma_map_single() work
From: Sergei Antonov @ 2022-08-12 15:48 UTC (permalink / raw)
To: netdev
Cc: Sergei Antonov, Jakub Kicinski, Pavel Skripkin, David S . Miller,
Guobin Huang, Paolo Abeni
dma_map_single() calls fail in moxart_mac_setup_desc_ring() and
moxart_mac_start_xmit() which leads to an incessant output of this:
[ 16.043925] moxart-ethernet 92000000.mac eth0: DMA mapping error
[ 16.050957] moxart-ethernet 92000000.mac eth0: DMA mapping error
[ 16.058229] moxart-ethernet 92000000.mac eth0: DMA mapping error
To make dma_map_single() work, inherit DMA masks from the platform device.
Signed-off-by: Sergei Antonov <saproj@gmail.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Pavel Skripkin <paskripkin@gmail.com>
CC: David S. Miller <davem@davemloft.net>
CC: Guobin Huang <huangguobin4@huawei.com>
CC: Paolo Abeni <pabeni@redhat.com>
---
drivers/net/ethernet/moxa/moxart_ether.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index a3214a762e4b..de99211d85c2 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -537,6 +537,10 @@ static int moxart_mac_probe(struct platform_device *pdev)
ndev->priv_flags |= IFF_UNICAST_FLT;
ndev->irq = irq;
+ /* Inherit the DMA masks from the platform device */
+ ndev->dev.dma_mask = p_dev->dma_mask;
+ ndev->dev.coherent_dma_mask = p_dev->coherent_dma_mask;
+
SET_NETDEV_DEV(ndev, &pdev->dev);
ret = register_netdev(ndev);
--
2.32.0
^ permalink raw reply related
* Re: [PATCH] net: sfp: reset fault retry counter on successful reinitialisation
From: Stefan Mahr @ 2022-08-12 15:45 UTC (permalink / raw)
To: Russell King (Oracle); +Cc: netdev, linux-kernel
In-Reply-To: <YvZswfC/JhkWmyBj@shell.armlinux.org.uk>
> On Fri, Aug 12, 2022 at 03:04:38PM +0200, Stefan Mahr wrote:
>> This patch resets the fault retry counter to the default value, if the
>> module reinitialisation was successful. Otherwise without resetting
>> the counter, five (N_FAULT/N_FAULT_INIT) single TX_FAULT events will
>> deactivate the module persistently.
>
> This is intentional - if a module keeps asserting its TX_FAULT status,
> then there is something wrong with it, and for an optical module it
> means that the laser is overloading (producing more output than it
> should.) That is a safety issue.
Yes, this behaviour is not changed by the patch. When TX_FAULT is true
persistently for 5 retries, the module will still be disabled.
>
> So, if the module keeps indicating that its laser is misbehaving the
> only right thing to do is to shut it down permanently, and have
> someone investigate.
>
> What issue are you seeing that needs a different behaviour?
>
In my case two different SFP modules (1000Base-BX) indicate short
TX_FAULT events for less than one second and only one per week -
sometimes more, sometimes less. So the idea was to reset the counter if
reinitialisation was successful, so rare single TX_FAULT events can be
ignored.
However, you are right: If a module reports TX_FAULT several times,
something serious may be wrong.
^ permalink raw reply
* [GIT PULL] virtio: fatures, fixes
From: Michael S. Tsirkin @ 2022-08-12 15:42 UTC (permalink / raw)
To: Linus Torvalds
Cc: kvm, virtualization, netdev, linux-kernel, alvaro.karsz,
colin.i.king, colin.king, dan.carpenter, david, elic, eperezma,
gautam.dawar, gshan, hdegoede, hulkci, jasowang, jiaming,
kangjie.xu, lingshan.zhu, liubo03, michael.christie, mst,
pankaj.gupta, peng.fan, quic_mingxue, robin.murphy, sgarzare,
suwan.kim027, syoshida, xieyongji, xuanzhuo, xuqiang36
The following changes since commit 3d7cb6b04c3f3115719235cc6866b10326de34cd:
Linux 5.19 (2022-07-31 14:03:01 -0700)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to 93e530d2a1c4c0fcce45e01ae6c5c6287a08d3e3:
vdpa/mlx5: Fix possible uninitialized return value (2022-08-11 10:00:36 -0400)
----------------------------------------------------------------
virtio: fatures, fixes
A huge patchset supporting vq resize using the
new vq reset capability.
Features, fixes, cleanups all over the place.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
----------------------------------------------------------------
Alvaro Karsz (1):
net: virtio_net: notifications coalescing support
Bo Liu (3):
virtio: Check dev_set_name() return value
vhost-vdpa: Call ida_simple_remove() when failed
virtio_vdpa: support the arg sizes of find_vqs()
Colin Ian King (1):
vDPA/ifcvf: remove duplicated assignment to pointer cfg
David Hildenbrand (1):
drivers/virtio: Clarify CONFIG_VIRTIO_MEM for unsupported architectures
Eli Cohen (3):
vdpa/mlx5: Implement susupend virtqueue callback
vdpa/mlx5: Support different address spaces for control and data
vdpa/mlx5: Fix possible uninitialized return value
Eugenio Pérez (4):
vdpa: Add suspend operation
vhost-vdpa: introduce SUSPEND backend feature bit
vhost-vdpa: uAPI to suspend the device
vdpa_sim: Implement suspend vdpa op
Jason Wang (2):
virtio_pmem: initialize provider_data through nd_region_desc
virtio_pmem: set device ready in probe()
Michael S. Tsirkin (1):
virtio: VIRTIO_HARDEN_NOTIFICATION is broken
Mike Christie (2):
vhost-scsi: Fix max number of virtqueues
vhost scsi: Allow user to control num virtqueues
Minghao Xue (2):
dt-bindings: virtio: mmio: add optional wakeup-source property
virtio_mmio: add support to set IRQ of a virtio device as wakeup source
Robin Murphy (1):
vdpa: Use device_iommu_capable()
Shigeru Yoshida (1):
virtio-blk: Avoid use-after-free on suspend/resume
Stefano Garzarella (11):
vringh: iterate on iotlb_translate to handle large translations
vdpa_sim_blk: use dev_dbg() to print errors
vdpa_sim_blk: limit the number of request handled per batch
vdpa_sim_blk: call vringh_complete_iotlb() also in the error path
vdpa_sim_blk: set number of address spaces and virtqueue groups
vdpa_sim: use max_iotlb_entries as a limit in vhost_iotlb_init
tools/virtio: fix build
vdpa_sim_blk: check if sector is 0 for commands other than read or write
vdpa_sim_blk: make vdpasim_blk_check_range usable by other requests
vdpa_sim_blk: add support for VIRTIO_BLK_T_FLUSH
vdpa_sim_blk: add support for discard and write-zeroes
Xie Yongji (5):
vduse: Remove unnecessary spin lock protection
vduse: Use memcpy_{to,from}_page() in do_bounce()
vduse: Support using userspace pages as bounce buffer
vduse: Support registering userspace memory for IOVA regions
vduse: Support querying information of IOVA regions
Xu Qiang (1):
vdpa/mlx5: Use eth_broadcast_addr() to assign broadcast address
Xuan Zhuo (44):
remoteproc: rename len of rpoc_vring to num
virtio_ring: remove the arg vq of vring_alloc_desc_extra()
virtio: record the maximum queue num supported by the device.
virtio: struct virtio_config_ops add callbacks for queue_reset
virtio_ring: update the document of the virtqueue_detach_unused_buf for queue reset
virtio_ring: extract the logic of freeing vring
virtio_ring: split vring_virtqueue
virtio_ring: introduce virtqueue_init()
virtio_ring: split: stop __vring_new_virtqueue as export symbol
virtio_ring: split: __vring_new_virtqueue() accept struct vring_virtqueue_split
virtio_ring: split: introduce vring_free_split()
virtio_ring: split: extract the logic of alloc queue
virtio_ring: split: extract the logic of alloc state and extra
virtio_ring: split: extract the logic of vring init
virtio_ring: split: extract the logic of attach vring
virtio_ring: split: introduce virtqueue_reinit_split()
virtio_ring: split: reserve vring_align, may_reduce_num
virtio_ring: split: introduce virtqueue_resize_split()
virtio_ring: packed: introduce vring_free_packed
virtio_ring: packed: extract the logic of alloc queue
virtio_ring: packed: extract the logic of alloc state and extra
virtio_ring: packed: extract the logic of vring init
virtio_ring: packed: extract the logic of attach vring
virtio_ring: packed: introduce virtqueue_reinit_packed()
virtio_ring: packed: introduce virtqueue_resize_packed()
virtio_ring: introduce virtqueue_resize()
virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
virtio: allow to unbreak/break virtqueue individually
virtio: queue_reset: add VIRTIO_F_RING_RESET
virtio_ring: struct virtqueue introduce reset
virtio_pci: struct virtio_pci_common_cfg add queue_reset
virtio_pci: introduce helper to get/set queue reset
virtio_pci: extract the logic of active vq for modern pci
virtio_pci: support VIRTIO_F_RING_RESET
virtio: find_vqs() add arg sizes
virtio_pci: support the arg sizes of find_vqs()
virtio_mmio: support the arg sizes of find_vqs()
virtio: add helper virtio_find_vqs_ctx_size()
virtio_net: set the default max ring size by find_vqs()
virtio_net: get ringparam by virtqueue_get_vring_max_size()
virtio_net: split free_unused_bufs()
virtio_net: support rx queue resize
virtio_net: support tx queue resize
virtio_net: support set_ringparam
Zhang Jiaming (1):
vdpa: ifcvf: Fix spelling mistake in comments
Zhu Lingshan (4):
vDPA/ifcvf: get_config_size should return a value no greater than dev implementation
vDPA/ifcvf: support userspace to query features and MQ of a management device
vDPA: !FEATURES_OK should not block querying device config space
vDPA: fix 'cast to restricted le16' warnings in vdpa.c
Documentation/devicetree/bindings/virtio/mmio.yaml | 4 +
arch/um/drivers/virtio_uml.c | 3 +-
drivers/block/virtio_blk.c | 24 +-
drivers/net/virtio_net.c | 325 +++++++-
drivers/nvdimm/virtio_pmem.c | 9 +-
drivers/platform/mellanox/mlxbf-tmfifo.c | 3 +
drivers/remoteproc/remoteproc_core.c | 4 +-
drivers/remoteproc/remoteproc_virtio.c | 13 +-
drivers/s390/virtio/virtio_ccw.c | 4 +
drivers/vdpa/ifcvf/ifcvf_base.c | 14 +-
drivers/vdpa/ifcvf/ifcvf_base.h | 2 +
drivers/vdpa/ifcvf/ifcvf_main.c | 144 ++--
drivers/vdpa/mlx5/core/mlx5_vdpa.h | 11 +
drivers/vdpa/mlx5/net/mlx5_vnet.c | 175 ++++-
drivers/vdpa/vdpa.c | 14 +-
drivers/vdpa/vdpa_sim/vdpa_sim.c | 18 +-
drivers/vdpa/vdpa_sim/vdpa_sim.h | 1 +
drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 176 ++++-
drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 +
drivers/vdpa/vdpa_user/iova_domain.c | 102 ++-
drivers/vdpa/vdpa_user/iova_domain.h | 8 +
drivers/vdpa/vdpa_user/vduse_dev.c | 180 +++++
drivers/vhost/scsi.c | 85 ++-
drivers/vhost/vdpa.c | 38 +-
drivers/vhost/vringh.c | 78 +-
drivers/virtio/Kconfig | 11 +-
drivers/virtio/virtio.c | 4 +-
drivers/virtio/virtio_mmio.c | 14 +-
drivers/virtio/virtio_pci_common.c | 32 +-
drivers/virtio/virtio_pci_common.h | 3 +-
drivers/virtio/virtio_pci_legacy.c | 8 +-
drivers/virtio/virtio_pci_modern.c | 153 +++-
drivers/virtio/virtio_pci_modern_dev.c | 39 +
drivers/virtio/virtio_ring.c | 814 +++++++++++++++------
drivers/virtio/virtio_vdpa.c | 18 +-
include/linux/mlx5/mlx5_ifc_vdpa.h | 8 +
include/linux/remoteproc.h | 4 +-
include/linux/vdpa.h | 4 +
include/linux/virtio.h | 10 +
include/linux/virtio_config.h | 40 +-
include/linux/virtio_pci_modern.h | 9 +
include/linux/virtio_ring.h | 10 -
include/uapi/linux/vduse.h | 47 ++
include/uapi/linux/vhost.h | 9 +
include/uapi/linux/vhost_types.h | 2 +
include/uapi/linux/virtio_config.h | 7 +-
include/uapi/linux/virtio_net.h | 34 +-
include/uapi/linux/virtio_pci.h | 2 +
tools/virtio/linux/kernel.h | 2 +-
tools/virtio/linux/vringh.h | 1 +
tools/virtio/virtio_test.c | 4 +-
51 files changed, 2171 insertions(+), 556 deletions(-)
^ permalink raw reply
* Re: [RFC net-next 3/4] ynl: add a sample python library
From: Edward Cree @ 2022-08-12 15:42 UTC (permalink / raw)
To: Stephen Hemminger, Jakub Kicinski
Cc: netdev, davem, edumazet, pabeni, sdf, jacob.e.keller, vadfed,
johannes, jiri, dsahern, fw, linux-doc
In-Reply-To: <20220811180452.13f06623@hermes.local>
On 12/08/2022 02:04, Stephen Hemminger wrote:
> On Wed, 10 Aug 2022 19:23:03 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
>
>> A very short and very incomplete generic python library.
>>
>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>
> It would be great if python had standard module for netlink.
> Then your code could just (re)use that.
> Something like mnl but for python.
There's pyroute2, that seemed alright when I used it for something
a few years back, and I think it has the pieces you need.
https://pyroute2.org/
-ed
^ permalink raw reply
* Re: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property
From: Andrew Lunn @ 2022-08-12 15:36 UTC (permalink / raw)
To: wei.fang
Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, f.fainelli, netdev, devicetree,
linux-kernel
In-Reply-To: <20220812145009.1229094-2-wei.fang@nxp.com>
On Sat, Aug 13, 2022 at 12:50:08AM +1000, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The hibernation mode of Atheros AR803x PHYs is default enabled.
> When the cable is unplugged, the PHY will enter hibernation
> mode and the PHY clock does down. For some MACs, it needs the
> clock to support it's logic. For instance, stmmac needs the PHY
> inputs clock is present for software reset completion. Therefore,
> It is reasonable to add a DT property to disable hibernation mode.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> Documentation/devicetree/bindings/net/qca,ar803x.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> index b3d4013b7ca6..d08431d79b83 100644
> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
> @@ -40,6 +40,12 @@ properties:
> Only supported on the AR8031.
> type: boolean
>
> + qca,disable-hibernation:
> + description: |
> + If set, the PHY will not enter hibernation mode when the cable is
> + unplugged.
> + type: boolean
The description itself needs indenting 2 space.
I would suggest you do what the bot suggests and install the dtschema
tools.
Andrew
^ permalink raw reply
* Re: [PATCH v4 net-next 3/6] drivers: net: dsa: add locked fdb entry flag to drivers
From: netdev @ 2022-08-12 15:33 UTC (permalink / raw)
To: Ido Schimmel
Cc: Vladimir Oltean, davem, kuba, netdev, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Eric Dumazet, Paolo Abeni, Jiri Pirko,
Ivan Vecera, Roopa Prabhu, Nikolay Aleksandrov, Shuah Khan,
Daniel Borkmann, linux-kernel, bridge, linux-kselftest
In-Reply-To: <YvTn847P/Ga3Ulb0@shredder>
On 2022-08-11 13:28, Ido Schimmel wrote:
>> >
>> > I'm talking about roaming, not forwarding. Let's say you have a locked
>> > entry with MAC X pointing to port Y. Now you get a packet with SMAC X
>> > from port Z which is unlocked. Will the FDB entry roam to port Z? I
>> > think it should, but at least in current implementation it seems that
>> > the "locked" flag will not be reset and having locked entries pointing
>> > to an unlocked port looks like a bug.
>> >
Yes, now I have tried to test with a case like this using the bridge and
have verified the locked entry pointing to an unlocked port, which I
agree seems to be a bug, which I will get fixed.
^ permalink raw reply
* [PATCH net 4/4] mlxsw: spectrum_ptp: Forbid PTP enablement only in RX or in TX
From: Petr Machata @ 2022-08-12 15:32 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: Ido Schimmel, Danielle Ratson, Amit Cohen, Richard Cochran,
Petr Machata, mlxsw
In-Reply-To: <cover.1660315448.git.petrm@nvidia.com>
From: Amit Cohen <amcohen@nvidia.com>
Currently mlxsw driver configures one global PTP configuration for all
ports. The reason is that the switch behaves like a transparent clock
between CPU port and front-panel ports. When time stamp is enabled in
any port, the hardware is configured to update the correction field. The
fact that the configuration of CPU port affects all the ports, makes the
correction field update to be global for all ports. Otherwise, user will
see odd values in the correction field, as the switch will update the
correction field in the CPU port, but not in all the front-panel ports.
The CPU port is relevant in both RX and TX, so to avoid problematic
configuration, forbid PTP enablement only in one direction, i.e., only in
RX or TX.
Without the change:
$ hwstamp_ctl -i swp1 -r 12 -t 0
current settings:
tx_type 0
rx_filter 0
new settings:
tx_type 0
rx_filter 2
$ echo $?
0
With the change:
$ hwstamp_ctl -i swp1 -r 12 -t 0
current settings:
tx_type 1
rx_filter 2
SIOCSHWTSTAMP failed: Invalid argument
Fixes: 08ef8bc825d96 ("mlxsw: spectrum_ptp: Support SIOCGHWTSTAMP, SIOCSHWTSTAMP ioctls")
Signed-off-by: Amit Cohen <amcohen@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index f32c83603b84..7b01b9c20722 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -1529,6 +1529,9 @@ mlxsw_sp2_ptp_get_message_types(const struct hwtstamp_config *config,
return -EINVAL;
}
+ if ((ing_types && !egr_types) || (!ing_types && egr_types))
+ return -EINVAL;
+
*p_ing_types = ing_types;
*p_egr_types = egr_types;
return 0;
--
2.35.3
^ permalink raw reply related
* [PATCH net 3/4] mlxsw: spectrum_ptp: Protect PTP configuration with a mutex
From: Petr Machata @ 2022-08-12 15:32 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: Ido Schimmel, Danielle Ratson, Amit Cohen, Richard Cochran,
Petr Machata, mlxsw
In-Reply-To: <cover.1660315448.git.petrm@nvidia.com>
From: Amit Cohen <amcohen@nvidia.com>
Currently the functions mlxsw_sp2_ptp_{configure, deconfigure}_port()
assume that they are called when RTNL is locked and they warn otherwise.
The deconfigure function can be called when port is removed, for example
as part of device reload, then there is no locked RTNL and the function
warns [1].
To avoid such case, do not assume that RTNL protects this code, add a
dedicated mutex instead. The mutex protects 'ptp_state->config' which
stores the existing global configuration in hardware. Use this mutex also
to protect the code which configures the hardware. Then, there will be
only one configuration in any time, which will be updated in 'ptp_state'
and a race will be avoided.
[1]:
RTNL: assertion failed at drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c (1600)
WARNING: CPU: 1 PID: 1583493 at drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c:1600 mlxsw_sp2_ptp_hwtstamp_set+0x2d3/0x300 [mlxsw_spectrum]
[...]
CPU: 1 PID: 1583493 Comm: devlink Not tainted5.19.0-rc8-custom-127022-gb371dffda095 #789
Hardware name: Mellanox Technologies Ltd.MSN3420/VMOD0005, BIOS 5.11 01/06/2019
RIP: 0010:mlxsw_sp2_ptp_hwtstamp_set+0x2d3/0x300[mlxsw_spectrum]
[...]
Call Trace:
<TASK>
mlxsw_sp_port_remove+0x7e/0x190 [mlxsw_spectrum]
mlxsw_sp_fini+0xd1/0x270 [mlxsw_spectrum]
mlxsw_core_bus_device_unregister+0x55/0x280 [mlxsw_core]
mlxsw_devlink_core_bus_device_reload_down+0x1c/0x30[mlxsw_core]
devlink_reload+0x1ee/0x230
devlink_nl_cmd_reload+0x4de/0x580
genl_family_rcv_msg_doit+0xdc/0x140
genl_rcv_msg+0xd7/0x1d0
netlink_rcv_skb+0x49/0xf0
genl_rcv+0x1f/0x30
netlink_unicast+0x22f/0x350
netlink_sendmsg+0x208/0x440
__sys_sendto+0xf0/0x140
__x64_sys_sendto+0x1b/0x20
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Fixes: 08ef8bc825d96 ("mlxsw: spectrum_ptp: Support SIOCGHWTSTAMP, SIOCSHWTSTAMP ioctls")
Reported-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Amit Cohen <amcohen@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 27 ++++++++++++++-----
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
index 2e0b704b8a31..f32c83603b84 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c
@@ -46,6 +46,7 @@ struct mlxsw_sp2_ptp_state {
* enabled.
*/
struct hwtstamp_config config;
+ struct mutex lock; /* Protects 'config' and HW configuration. */
};
struct mlxsw_sp1_ptp_key {
@@ -1374,6 +1375,7 @@ struct mlxsw_sp_ptp_state *mlxsw_sp2_ptp_init(struct mlxsw_sp *mlxsw_sp)
goto err_ptp_traps_set;
refcount_set(&ptp_state->ptp_port_enabled_ref, 0);
+ mutex_init(&ptp_state->lock);
return &ptp_state->common;
err_ptp_traps_set:
@@ -1388,6 +1390,7 @@ void mlxsw_sp2_ptp_fini(struct mlxsw_sp_ptp_state *ptp_state_common)
ptp_state = mlxsw_sp2_ptp_state(mlxsw_sp);
+ mutex_destroy(&ptp_state->lock);
mlxsw_sp_ptp_traps_unset(mlxsw_sp);
kfree(ptp_state);
}
@@ -1461,7 +1464,10 @@ int mlxsw_sp2_ptp_hwtstamp_get(struct mlxsw_sp_port *mlxsw_sp_port,
ptp_state = mlxsw_sp2_ptp_state(mlxsw_sp_port->mlxsw_sp);
+ mutex_lock(&ptp_state->lock);
*config = ptp_state->config;
+ mutex_unlock(&ptp_state->lock);
+
return 0;
}
@@ -1574,8 +1580,6 @@ static int mlxsw_sp2_ptp_configure_port(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp2_ptp_state *ptp_state;
int err;
- ASSERT_RTNL();
-
ptp_state = mlxsw_sp2_ptp_state(mlxsw_sp_port->mlxsw_sp);
if (refcount_inc_not_zero(&ptp_state->ptp_port_enabled_ref))
@@ -1597,8 +1601,6 @@ static int mlxsw_sp2_ptp_deconfigure_port(struct mlxsw_sp_port *mlxsw_sp_port,
struct mlxsw_sp2_ptp_state *ptp_state;
int err;
- ASSERT_RTNL();
-
ptp_state = mlxsw_sp2_ptp_state(mlxsw_sp_port->mlxsw_sp);
if (!refcount_dec_and_test(&ptp_state->ptp_port_enabled_ref))
@@ -1618,16 +1620,20 @@ static int mlxsw_sp2_ptp_deconfigure_port(struct mlxsw_sp_port *mlxsw_sp_port,
int mlxsw_sp2_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
struct hwtstamp_config *config)
{
+ struct mlxsw_sp2_ptp_state *ptp_state;
enum hwtstamp_rx_filters rx_filter;
struct hwtstamp_config new_config;
u16 new_ing_types, new_egr_types;
bool ptp_enabled;
int err;
+ ptp_state = mlxsw_sp2_ptp_state(mlxsw_sp_port->mlxsw_sp);
+ mutex_lock(&ptp_state->lock);
+
err = mlxsw_sp2_ptp_get_message_types(config, &new_ing_types,
&new_egr_types, &rx_filter);
if (err)
- return err;
+ goto err_get_message_types;
new_config.flags = config->flags;
new_config.tx_type = config->tx_type;
@@ -1640,11 +1646,11 @@ int mlxsw_sp2_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
err = mlxsw_sp2_ptp_configure_port(mlxsw_sp_port, new_ing_types,
new_egr_types, new_config);
if (err)
- return err;
+ goto err_configure_port;
} else if (!new_ing_types && !new_egr_types && ptp_enabled) {
err = mlxsw_sp2_ptp_deconfigure_port(mlxsw_sp_port, new_config);
if (err)
- return err;
+ goto err_deconfigure_port;
}
mlxsw_sp_port->ptp.ing_types = new_ing_types;
@@ -1652,8 +1658,15 @@ int mlxsw_sp2_ptp_hwtstamp_set(struct mlxsw_sp_port *mlxsw_sp_port,
/* Notify the ioctl caller what we are actually timestamping. */
config->rx_filter = rx_filter;
+ mutex_unlock(&ptp_state->lock);
return 0;
+
+err_deconfigure_port:
+err_configure_port:
+err_get_message_types:
+ mutex_unlock(&ptp_state->lock);
+ return err;
}
int mlxsw_sp2_ptp_get_ts_info(struct mlxsw_sp *mlxsw_sp,
--
2.35.3
^ permalink raw reply related
* [PATCH net 2/4] mlxsw: spectrum: Clear PTP configuration after unregistering the netdevice
From: Petr Machata @ 2022-08-12 15:32 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: Ido Schimmel, Danielle Ratson, Amit Cohen, Richard Cochran,
Petr Machata, mlxsw
In-Reply-To: <cover.1660315448.git.petrm@nvidia.com>
From: Amit Cohen <amcohen@nvidia.com>
Currently as part of removing port, PTP API is called to clear the
existing configuration and set the 'rx_filter' and 'tx_type' to zero.
The clearing is done before unregistering the netdevice, which means that
there is a window of time in which the user can reconfigure PTP in the
port, and this configuration will not be cleared.
Reorder the operations, clear PTP configuration after unregistering the
netdevice.
Fixes: 8748642751ede ("mlxsw: spectrum: PTP: Support SIOCGHWTSTAMP, SIOCSHWTSTAMP ioctls")
Signed-off-by: Amit Cohen <amcohen@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 1e240cdd9cbd..30c7b0e15721 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1897,9 +1897,9 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u16 local_port)
cancel_delayed_work_sync(&mlxsw_sp_port->periodic_hw_stats.update_dw);
cancel_delayed_work_sync(&mlxsw_sp_port->ptp.shaper_dw);
- mlxsw_sp_port_ptp_clear(mlxsw_sp_port);
mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
unregister_netdev(mlxsw_sp_port->dev); /* This calls ndo_stop */
+ mlxsw_sp_port_ptp_clear(mlxsw_sp_port);
mlxsw_sp_port_vlan_classification_set(mlxsw_sp_port, true, true);
mlxsw_sp->ports[local_port] = NULL;
mlxsw_sp_port_vlan_flush(mlxsw_sp_port, true);
--
2.35.3
^ permalink raw reply related
* [PATCH net 1/4] mlxsw: spectrum_ptp: Fix compilation warnings
From: Petr Machata @ 2022-08-12 15:32 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: Ido Schimmel, Danielle Ratson, Amit Cohen, Richard Cochran,
Petr Machata, mlxsw, kernel test robot
In-Reply-To: <cover.1660315448.git.petrm@nvidia.com>
From: Amit Cohen <amcohen@nvidia.com>
In case that 'CONFIG_PTP_1588_CLOCK' is not enabled in the config file,
there are implementations for the functions
mlxsw_{sp,sp2}_ptp_txhdr_construct() as part of 'spectrum_ptp.h'. In this
case, they should be defined as 'static' as they are not supposed to be
used out of this file. Make the functions 'static', otherwise the following
warnings are returned:
"warning: no previous prototype for 'mlxsw_sp_ptp_txhdr_construct'"
"warning: no previous prototype for 'mlxsw_sp2_ptp_txhdr_construct'"
In addition, make the functions 'inline' for case that 'spectrum_ptp.h'
will be included anywhere else and the functions would probably not be
used, so compilation warnings about unused static will be returned.
Fixes: 24157bc69f45 ("mlxsw: Send PTP packets as data packets to overcome a limitation")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Amit Cohen <amcohen@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_ptp.h | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
index 2d1628fdefc1..a8b88230959a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h
@@ -171,10 +171,11 @@ static inline void mlxsw_sp1_get_stats(struct mlxsw_sp_port *mlxsw_sp_port,
{
}
-int mlxsw_sp_ptp_txhdr_construct(struct mlxsw_core *mlxsw_core,
- struct mlxsw_sp_port *mlxsw_sp_port,
- struct sk_buff *skb,
- const struct mlxsw_tx_info *tx_info)
+static inline int
+mlxsw_sp_ptp_txhdr_construct(struct mlxsw_core *mlxsw_core,
+ struct mlxsw_sp_port *mlxsw_sp_port,
+ struct sk_buff *skb,
+ const struct mlxsw_tx_info *tx_info)
{
return -EOPNOTSUPP;
}
@@ -231,10 +232,11 @@ static inline int mlxsw_sp2_ptp_get_ts_info(struct mlxsw_sp *mlxsw_sp,
return mlxsw_sp_ptp_get_ts_info_noptp(info);
}
-int mlxsw_sp2_ptp_txhdr_construct(struct mlxsw_core *mlxsw_core,
- struct mlxsw_sp_port *mlxsw_sp_port,
- struct sk_buff *skb,
- const struct mlxsw_tx_info *tx_info)
+static inline int
+mlxsw_sp2_ptp_txhdr_construct(struct mlxsw_core *mlxsw_core,
+ struct mlxsw_sp_port *mlxsw_sp_port,
+ struct sk_buff *skb,
+ const struct mlxsw_tx_info *tx_info)
{
return -EOPNOTSUPP;
}
--
2.35.3
^ permalink raw reply related
* [PATCH net 0/4] mlxsw: Fixes for PTP support
From: Petr Machata @ 2022-08-12 15:31 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: Ido Schimmel, Danielle Ratson, Amit Cohen, Richard Cochran,
Petr Machata, mlxsw
This set fixes several issues in mlxsw PTP code.
- Patch #1 fixes compilation warnings.
- Patch #2 adjusts the order of operation during cleanup, thereby
closing the window after PTP state was already cleaned in the ASIC
for the given port, but before the port is removed, when the user
could still in theory make changes to the configuration.
- Patch #3 protects the PTP configuration with a custom mutex, instead
of relying on RTNL, which is not held in all access paths.
- Patch #4 forbids enablement of PTP only in RX or only in TX. The
driver implicitly assumed this would be the case, but neglected to
sanitize the configuration.
Amit Cohen (4):
mlxsw: spectrum_ptp: Fix compilation warnings
mlxsw: spectrum: Clear PTP configuration after unregistering the
netdevice
mlxsw: spectrum_ptp: Protect PTP configuration with a mutex
mlxsw: spectrum_ptp: Forbid PTP enablement only in RX or in TX
.../net/ethernet/mellanox/mlxsw/spectrum.c | 2 +-
.../ethernet/mellanox/mlxsw/spectrum_ptp.c | 30 ++++++++++++++-----
.../ethernet/mellanox/mlxsw/spectrum_ptp.h | 18 ++++++-----
3 files changed, 34 insertions(+), 16 deletions(-)
--
2.35.3
^ permalink raw reply
* Re: [PATCH RFC net-next 0/3] net: vlan: fix bridge binding behavior and add selftests
From: Sevinj Aghayeva @ 2022-08-12 15:30 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, aroulin, sbrivio, roopa, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel, bridge
In-Reply-To: <94ec6182-0804-7a0e-dcba-42655ff19884@blackwall.org>
On Wed, Aug 10, 2022 at 4:54 AM Nikolay Aleksandrov <razor@blackwall.org> wrote:
>
> On 10/08/2022 06:11, Sevinj Aghayeva wrote:
> > When bridge binding is enabled for a vlan interface, it is expected
> > that the link state of the vlan interface will track the subset of the
> > ports that are also members of the corresponding vlan, rather than
> > that of all ports.
> >
> > Currently, this feature works as expected when a vlan interface is
> > created with bridge binding enabled:
> >
> > ip link add link br name vlan10 type vlan id 10 protocol 802.1q \
> > bridge_binding on
> >
> > However, the feature does not work when a vlan interface is created
> > with bridge binding disabled, and then enabled later:
> >
> > ip link add link br name vlan10 type vlan id 10 protocol 802.1q \
> > bridge_binding off
> > ip link set vlan10 type vlan bridge_binding on
> >
> > After these two commands, the link state of the vlan interface
> > continues to track that of all ports, which is inconsistent and
> > confusing to users. This series fixes this bug and introduces two
> > tests for the valid behavior.
> >
> > Sevinj Aghayeva (3):
> > net: core: export call_netdevice_notifiers_info
> > net: 8021q: fix bridge binding behavior for vlan interfaces
> > selftests: net: tests for bridge binding behavior
> >
> > include/linux/netdevice.h | 2 +
> > net/8021q/vlan.h | 2 +-
> > net/8021q/vlan_dev.c | 25 ++-
> > net/core/dev.c | 7 +-
> > tools/testing/selftests/net/Makefile | 1 +
> > .../selftests/net/bridge_vlan_binding_test.sh | 143 ++++++++++++++++++
> > 6 files changed, 172 insertions(+), 8 deletions(-)
> > create mode 100755 tools/testing/selftests/net/bridge_vlan_binding_test.sh
> >
>
> Hi,
> NETDEV_CHANGE event is already propagated when the vlan changes flags,
> NETDEV_CHANGEUPPER is used when the devices' relationship changes not their flags.
> The only problem you have to figure out is that the flag has changed. The fix itself
> must be done within the bridge, not 8021q. You can figure it out based on current bridge
> loose binding state and the vlan's changed state, again in the bridge's NETDEV_CHANGE
> handler. Unfortunately the proper fix is much more involved and will need new
> infra, you'll have to track the loose binding vlans in the bridge. To do that you should
> add logic that reflects the current vlans' loose binding state *only* for vlans that also
> exist in the bridge, the rest which are upper should be carrier off if they have the loose
> binding flag set.
>
> Alternatively you can add a new NETDEV_ notifier (using something similar to struct netdev_notifier_pre_changeaddr_info)
> and add link type-specific space (e.g. union of link type-specific structs) in the struct which will contain
> what changed for 8021q and will be properly interpreted by the bridge. The downside is that we'll generate
> 2 notifications when changing the loose binding flag, but on the bright side won't have to track anything
> in the bridge, just handle the new notifier type. This might be the easiest path, the fix is still in
> the bridge though, the 8021q module just needs to fill in the new struct and emit the notification on
> any loose binding changes, the bridge must decide if it should process it (i.e. based on upper/lower
> relationship). Such notifier can be also re-used by other link types to propagate link-type specific
> changes.
Hi Nik,
Can you please clarify the following?
1) should the new NETDEV_ notifier be about the vlan device and not
the bridge? That is, should I handle it in br_device_event?
2) is it still okay to export call_netdevice_notifiers_info or should
i write a new function for this?
The answers to the above wasn't clear to me, but I came up with the
following patch anyway, so perhaps you can also comment on it. I'm
pasting it inline; this is against 5.19.
Thanks!
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2563d30736e9..c63205eb1f72 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2762,6 +2762,7 @@ enum netdev_cmd {
NETDEV_UNREGISTER,
NETDEV_CHANGEMTU, /* notify after mtu change happened */
NETDEV_CHANGEADDR, /* notify after the address change */
+ NETDEV_CHANGEUPPERFLAGS,
NETDEV_PRE_CHANGEADDR, /* notify before the address change */
NETDEV_GOING_DOWN,
NETDEV_CHANGENAME,
@@ -2837,6 +2838,12 @@ struct netdev_notifier_changelowerstate_info {
void *lower_state_info; /* is lower dev state */
};
+struct netdev_notifier_changeupperflags_info {
+ struct netdev_notifier_info info; /* must be first */
+ struct net_device *upper_dev;
+ bool vlan_bridge_binding;
+};
+
struct netdev_notifier_pre_changeaddr_info {
struct netdev_notifier_info info; /* must be first */
const unsigned char *dev_addr;
@@ -2898,6 +2905,8 @@ netdev_notifier_info_to_extack(const struct
netdev_notifier_info *info)
}
int call_netdevice_notifiers(unsigned long val, struct net_device *dev);
+int call_netdevice_notifiers_info(unsigned long val,
+ struct netdev_notifier_info *info);
extern rwlock_t dev_base_lock; /* Device list lock */
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 5eaf38875554..71947cdcfaaa 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -130,7 +130,7 @@ void vlan_dev_set_ingress_priority(const struct
net_device *dev,
int vlan_dev_set_egress_priority(const struct net_device *dev,
u32 skb_prio, u16 vlan_prio);
void vlan_dev_free_egress_priority(const struct net_device *dev);
-int vlan_dev_change_flags(const struct net_device *dev, u32 flag, u32 mask);
+int vlan_dev_change_flags(struct net_device *dev, u32 flag, u32 mask);
void vlan_dev_get_realdev_name(const struct net_device *dev, char *result,
size_t size);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 839f2020b015..68da3901dfb0 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -208,11 +208,18 @@ int vlan_dev_set_egress_priority(const struct
net_device *dev,
return 0;
}
+static inline bool netif_is_bridge(const struct net_device *dev)
+{
+ return dev->rtnl_link_ops &&
+ !strcmp(dev->rtnl_link_ops->kind, "bridge");
+}
+
/* Flags are defined in the vlan_flags enum in
* include/uapi/linux/if_vlan.h file.
*/
-int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
+int vlan_dev_change_flags(struct net_device *dev, u32 flags, u32 mask)
{
+ struct netdev_notifier_changeupperflags_info info;
struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
u32 old_flags = vlan->flags;
@@ -223,19 +230,33 @@ int vlan_dev_change_flags(const struct
net_device *dev, u32 flags, u32 mask)
vlan->flags = (old_flags & ~mask) | (flags & mask);
- if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
+ if (!netif_running(dev))
+ return 0;
+
+ if ((vlan->flags ^ old_flags) & VLAN_FLAG_GVRP) {
if (vlan->flags & VLAN_FLAG_GVRP)
vlan_gvrp_request_join(dev);
else
vlan_gvrp_request_leave(dev);
}
- if (netif_running(dev) && (vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
+ if ((vlan->flags ^ old_flags) & VLAN_FLAG_MVRP) {
if (vlan->flags & VLAN_FLAG_MVRP)
vlan_mvrp_request_join(dev);
else
vlan_mvrp_request_leave(dev);
}
+
+ if ((vlan->flags ^ old_flags) & VLAN_FLAG_BRIDGE_BINDING &&
+ netif_is_bridge(vlan->real_dev)) {
+ info.info.dev = vlan->real_dev;
+ info.upper_dev = dev;
+ info.vlan_bridge_binding =
+ !!(vlan->flags & VLAN_FLAG_BRIDGE_BINDING);
+ call_netdevice_notifiers_info(NETDEV_CHANGEUPPERFLAGS,
+ &info.info);
+ }
+
return 0;
}
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 0f5e75ccac79..cbcb0877d4a4 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1718,6 +1718,7 @@ static void nbp_vlan_set_vlan_dev_state(struct
net_bridge_port *p, u16 vid)
/* Must be protected by RTNL. */
int br_vlan_bridge_event(struct net_device *dev, unsigned long event,
void *ptr)
{
+ struct netdev_notifier_changeupperflags_info *flags_info;
struct netdev_notifier_changeupper_info *info;
struct net_bridge *br = netdev_priv(dev);
int vlcmd = 0, ret = 0;
@@ -1739,7 +1740,11 @@ int br_vlan_bridge_event(struct net_device
*dev, unsigned long event, void *ptr)
info = ptr;
br_vlan_upper_change(dev, info->upper_dev, info->linking);
break;
-
+ case NETDEV_CHANGEUPPERFLAGS:
+ flags_info = ptr;
+ br_vlan_upper_change(dev, flags_info->upper_dev,
+ flags_info->vlan_bridge_binding);
+ break;
case NETDEV_CHANGE:
case NETDEV_UP:
if (!br_opt_get(br, BROPT_VLAN_BRIDGE_BINDING))
diff --git a/net/core/dev.c b/net/core/dev.c
index 30a1603a7225..bc8640d77d83 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -160,8 +160,6 @@ struct list_head ptype_base[PTYPE_HASH_SIZE] __read_mostly;
struct list_head ptype_all __read_mostly; /* Taps */
static int netif_rx_internal(struct sk_buff *skb);
-static int call_netdevice_notifiers_info(unsigned long val,
- struct netdev_notifier_info *info);
static int call_netdevice_notifiers_extack(unsigned long val,
struct net_device *dev,
struct netlink_ext_ack *extack);
@@ -1624,7 +1622,7 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
N(POST_INIT) N(RELEASE) N(NOTIFY_PEERS) N(JOIN) N(CHANGEUPPER)
N(RESEND_IGMP) N(PRECHANGEMTU) N(CHANGEINFODATA) N(BONDING_INFO)
N(PRECHANGEUPPER) N(CHANGELOWERSTATE) N(UDP_TUNNEL_PUSH_INFO)
- N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
+ N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN) N(CHANGEUPPERFLAGS)
N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
N(PRE_CHANGEADDR) N(OFFLOAD_XSTATS_ENABLE) N(OFFLOAD_XSTATS_DISABLE)
@@ -1927,8 +1925,8 @@ static void
move_netdevice_notifiers_dev_net(struct net_device *dev,
* are as for raw_notifier_call_chain().
*/
-static int call_netdevice_notifiers_info(unsigned long val,
- struct netdev_notifier_info *info)
+int call_netdevice_notifiers_info(unsigned long val,
+ struct netdev_notifier_info *info)
{
struct net *net = dev_net(info->dev);
int ret;
@@ -1944,6 +1942,7 @@ static int
call_netdevice_notifiers_info(unsigned long val,
return ret;
return raw_notifier_call_chain(&netdev_chain, val, info);
}
+EXPORT_SYMBOL(call_netdevice_notifiers_info);
/**
* call_netdevice_notifiers_info_robust - call per-netns notifier blocks
>
> Both of these avoid any direct dependencies between the bridge and 8021q. Any other suggestions that
> are simpler, avoid direct dependencies and solve the issue in a generic way would be appreciated.
>
> Just be careful about introducing too much unnecessary processing because we
> can have lots of vlan devices in a system.
>
> Cheers,
> Nik
--
Sevinj.Aghayeva
^ permalink raw reply related
* Re: [PATCH] configure: Define _GNU_SOURCE when checking for setns
From: patchwork-bot+netdevbpf @ 2022-08-12 15:30 UTC (permalink / raw)
To: Khem Raj; +Cc: netdev
In-Reply-To: <20220811053440.778649-1-raj.khem@gmail.com>
Hello:
This patch was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:
On Wed, 10 Aug 2022 22:34:40 -0700 you wrote:
> glibc defines this function only as gnu extention
>
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
> configure | 1 +
> 1 file changed, 1 insertion(+)
Here is the summary with links:
- configure: Define _GNU_SOURCE when checking for setns
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=d5fe96ab7092
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan_rx()
From: Greg KH @ 2022-08-12 15:27 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: johannes berg, david s. miller, eric dumazet, jakub kicinski,
paolo abeni, netdev, syzbot+6cb476b7c69916a0caca, linux-wireless,
linux-kernel, syzbot+f9acff9bf08a845f225d,
syzbot+9250865a55539d384347, linux-kernel-mentees
In-Reply-To: <18292791718.88f48d22175003.6675210189148271554@siddh.me>
On Fri, Aug 12, 2022 at 08:03:05PM +0530, Siddh Raman Pant wrote:
> On Fri, 12 Aug 2022 17:45:58 +0530 Greg KH wrote:
> > The merge window is for new features to be added, bugfixes can be merged
> > at any point in time, but most maintainers close their trees until after
> > the merge window is closed before accepting new fixes, like this one.
> >
> > So just relax, wait another week or so, and if there's no response,
> > resend it then.
> >
>
> Okay, sure.
>
> > Personally, this patch seems very incorrect, but hey, I'm not the wifi
> > subsystem maintainer :)
>
> Why do you think so?
rcu just delays freeing of an object, you might just be delaying the
race condition. Just moving a single object to be freed with rcu feels
very odd if you don't have another reference somewhere.
Anyway, I don't know this codebase, so I could be totally wrong.
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/3] can: ctucanfd: add HW timestamps to RX and error CAN frames
From: Pavel Pisa @ 2022-08-12 15:19 UTC (permalink / raw)
To: Vincent Mailhol, Matej Vasilevski, Pavel Hronek
Cc: Ondrej Ille, Wolfgang Grandegger, Marc Kleine-Budde,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, linux-can, netdev, devicetree,
Jiri Novak, Oliver Hartkopp
In-Reply-To: <CAMZ6Rq+EehdX8Kkg_430tzbE072Fm0PXbzgSqBzeDygTZqzBLA@mail.gmail.com>
Hello Vincent,
On Friday 12 of August 2022 16:35:30 Vincent Mailhol wrote:
> Hi Pavel,
>
> On Tue. 2 Aug. 2022 at 16:38, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
> > Hello Vincent,
> >
> > thanks much for review. I am adding some notices to Tx timestamps
> > after your comments
> >
> > On Tuesday 02 of August 2022 05:43:38 Vincent Mailhol wrote:
> > > I just send a series last week which a significant amount of changes
> > > for CAN timestamping tree-wide:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/
> > >comm it/?id=12a18d79dc14c80b358dbd26461614b97f2ea4a6
> > >
> > > I suggest you have a look at this series and harmonize it with the new
> > > features (e.g. Hardware TX timestamp).
> > >
> > > On Tue. 2 Aug. 2022 at 03:52, Matej Vasilevski
> >
> > ...
> >
> > > > +static int ctucan_hwtstamp_set(struct net_device *dev, struct ifreq
> > > > *ifr) +{
> > > > + struct ctucan_priv *priv = netdev_priv(dev);
> > > > + struct hwtstamp_config cfg;
> > > > +
> > > > + if (!priv->timestamp_possible)
> > > > + return -EOPNOTSUPP;
> > > > +
> > > > + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
> > > > + return -EFAULT;
> > > > +
> > > > + if (cfg.flags)
> > > > + return -EINVAL;
> > > > +
> > > > + if (cfg.tx_type != HWTSTAMP_TX_OFF)
> > > > + return -ERANGE;
> > >
> > > I have a great news: your driver now also support hardware TX
> > > timestamps:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/
> > >comm it/?id=8bdd1112edcd3edce2843e03826204a84a61042d
> > >
> > > > +
> > > > + switch (cfg.rx_filter) {
> > > > + case HWTSTAMP_FILTER_NONE:
> > > > + priv->timestamp_enabled = false;
> >
> > ...
> >
> > > > +
> > > > + cfg.flags = 0;
> > > > + cfg.tx_type = HWTSTAMP_TX_OFF;
> > >
> > > Hardware TX timestamps are now supported (c.f. supra).
> > >
> > > > + cfg.rx_filter = priv->timestamp_enabled ? HWTSTAMP_FILTER_ALL
> > > > : HWTSTAMP_FILTER_NONE; + return copy_to_user(ifr->ifr_data,
> > > > &cfg, sizeof(cfg)) ? -EFAULT : 0; +}
> > > > +
> > > > +static int ctucan_ioctl(struct net_device *dev, struct ifreq *ifr,
> > > > int cmd)
> > >
> > > Please consider using the generic function can_eth_ioctl_hwts()
> > > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/
> > >comm it/?id=90f942c5a6d775bad1be33ba214755314105da4a
> > >
> > > > +{
> >
> > ...
> >
> > > > + info->so_timestamping |= SOF_TIMESTAMPING_RX_HARDWARE |
> > > > + SOF_TIMESTAMPING_RAW_HARDWARE;
> > > > + info->tx_types = BIT(HWTSTAMP_TX_OFF);
> > >
> > > Hardware TX timestamps are now supported (c.f. supra).
> > >
> > > > + info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
> > > > + BIT(HWTSTAMP_FILTER_ALL);
> >
> > I am not sure if it is good idea to report support for hardware
> > TX timestamps by all drivers. Precise hardware Tx timestamps
> > are important for some CAN applications but they require to be
> > exactly/properly aligned with Rx timestamps.
> >
> > Only some CAN (FD) controllers really support that feature.
> > For M-CAN and some others it is realized as another event
> > FIFO in addition to Tx and Rx FIFOs.
> >
> > For CTU CAN FD, we have decided that we do not complicate design
> > and driver by separate events channel. We have configurable
> > and possibly large Rx FIFO depth which is logical to use for
> > analyzer mode and we can use loopback to receive own messages
> > timestamped same way as external received ones.
> >
> > See 2.14.1 Loopback mode
> > SETTINGS[ILBP]=1.
> >
> > in the datasheet
> >
> > http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/doc/Datasheet.pdf
> >
> > There is still missing information which frames are received
> > locally and from which buffer they are in the Rx message format,
> > but we plan to add that into VHDL design.
> >
> > In such case, we can switch driver mode and release Tx buffers
> > only after corresponding message is read from Rx FIFO and
> > fill exact finegrain (10 ns in our current design) timestamps
> > to the echo skb. The order of received messages will be seen
> > exactly mathing the wire order for both transmitted and received
> > messages then. Which I consider as proper solution for the
> > most applications including CAN bus analyzers.
> >
> > So I consider to report HW Tx timestamps for cases where exact,
> > precise timestamping is not available for loopback messages
> > as problematic because you cannot distinguish if you talk
> > with driver and HW with real/precise timestamps support
> > or only dummy implementation to make some tools happy.
>
> Thank you for the explanation. I did not know about the nuance about
> those different hardware timestamps.
>
> So if I understand correctly, your hardware can deliver two types of
> hardware timestamps:
>
> - The "real" one: fine grained with 10 ns precision when the frame
> is actually sent
>
> - The "dummy" one: less precise timestamp generated when there is an
> event on the device’s Rx or Tx FIFO.
>
> Is this correct?
>
> Are the "real" and the "dummy" timestamps synced on the same quartz?
>
> What is the precision of the "dummy" timestamp? If it in the order of
> magnitude of 10µs? For many use cases, this is enough. 10µs represents
> roughly a dozen of time quata (more or less depending on the bitrate
> and its prescaler).
> Actually, I never saw hardware with a timestamp precision below 1µs
> (not saying those don't exist, just that I never encountered them).
>
> I am not against what you propose. But my suggestion would be rather
> to report both TX and RX timestamps and just document the precision
> (i.e. TX has precision with an order of magnitude of 10µs and RX has
> precision of 10 ns).
>
> At the end, I let you decide what works the best for you. Just keep in
> mind that the micro second precision is already a great achievement
> and not many people need the 10 nano second (especially for CAN).
>
> P.S.: I am on holiday, my answers might be delayed :)
I am leaving off the Internet for next week as well now...
My discussion has been reaction to your information about your
CTU CAN FD change, but may it be I have lost the track.
> > On Tuesday 02 of August 2022 05:43:38 Vincent Mailhol wrote:
> > > I have a great news: your driver now also support hardware TX
> > > timestamps:
Our actual/mainline driver actually does not support neither Rx nor Tx
timestamps. Matej Vasilevski has prepared and sent to review patches
adding Rx timestamping (10 ns resolution for our actual designs).
He has rebased his changes above yours... CTU CAN FD hardware
supports such timestamping for many years... probably preceding 2.0
IP core version.
But even when this patch is clean up and accepted into mainline,
CTU CAN FD driver will not support hardware Tx timestams, may it
be software ones are implemented in generic CAN echo code, not checked
now... So if your change add report of HW Tx stamps then it would be
problem to distinguish situation when we implement hardware Tx timestamps.
The rest of the previous text is how to implement precise Tx timestamps
on other and our controller design. We do not have separate queue
to report Tx timestamps for successfully sent frames. But we can
enable copy of sent Tx frames to Rx FIFO so there is a way how
to achieve that. But there is still minor design detail that
we need to mark such frames as echo of local Tx in Rx FIFO queue
and ideally add there even number of the Tx buffer or even some
user provided information from some Tx buffer filed to distinguish
that such frames should be reported through echo and ensure that
they are not reported to that client who has sent them etc...
But there are our implementation details...
But what worries me, is your statement that HW Tx timestamps
are already reported as available on CTU CAN FD by your patch,
if I understood your reply well.
Best wishes,
Pavel
--
Pavel Pisa
phone: +420 603531357
e-mail: pisa@cmp.felk.cvut.cz
Department of Control Engineering FEE CVUT
Karlovo namesti 13, 121 35, Prague 2
university: http://control.fel.cvut.cz/
personal: http://cmp.felk.cvut.cz/~pisa
projects: https://www.openhub.net/accounts/ppisa
CAN related:http://canbus.pages.fel.cvut.cz/
RISC-V education: https://comparch.edu.cvut.cz/
Open Technologies Research Education and Exchange Services
https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home
^ permalink raw reply
* Re: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property
From: Rob Herring @ 2022-08-12 15:13 UTC (permalink / raw)
To: wei.fang
Cc: andrew, pabeni, robh+dt, f.fainelli, hkallweit1, netdev,
krzysztof.kozlowski+dt, linux-kernel, davem, linux, edumazet,
kuba, devicetree
In-Reply-To: <20220812145009.1229094-2-wei.fang@nxp.com>
On Sat, 13 Aug 2022 00:50:08 +1000, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The hibernation mode of Atheros AR803x PHYs is default enabled.
> When the cable is unplugged, the PHY will enter hibernation
> mode and the PHY clock does down. For some MACs, it needs the
> clock to support it's logic. For instance, stmmac needs the PHY
> inputs clock is present for software reset completion. Therefore,
> It is reasonable to add a DT property to disable hibernation mode.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> Documentation/devicetree/bindings/net/qca,ar803x.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/net/qca,ar803x.yaml:46:5: [error] syntax error: could not find expected ':' (syntax)
dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/net/qca,ar803x.example.dts'
Documentation/devicetree/bindings/net/qca,ar803x.yaml:46:5: could not find expected ':'
make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/net/qca,ar803x.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/net/qca,ar803x.yaml:46:5: could not find expected ':'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/qca,ar803x.yaml: ignoring, error parsing file
make: *** [Makefile:1404: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH] net: sfp: reset fault retry counter on successful reinitialisation
From: Russell King (Oracle) @ 2022-08-12 15:07 UTC (permalink / raw)
To: Stefan Mahr; +Cc: netdev, linux-kernel
In-Reply-To: <20220812130438.140434-1-dac922@gmx.de>
On Fri, Aug 12, 2022 at 03:04:38PM +0200, Stefan Mahr wrote:
> This patch resets the fault retry counter to the default value, if the
> module reinitialisation was successful. Otherwise without resetting
> the counter, five (N_FAULT/N_FAULT_INIT) single TX_FAULT events will
> deactivate the module persistently.
This is intentional - if a module keeps asserting its TX_FAULT status,
then there is something wrong with it, and for an optical module it
means that the laser is overloading (producing more output than it
should.) That is a safety issue.
So, if the module keeps indicating that its laser is misbehaving the
only right thing to do is to shut it down permanently, and have
someone investigate.
What issue are you seeing that needs a different behaviour?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v2 1/3] can: ctucanfd: add HW timestamps to RX and error CAN frames
From: Vincent Mailhol @ 2022-08-12 14:35 UTC (permalink / raw)
To: Pavel Pisa
Cc: Matej Vasilevski, Ondrej Ille, Wolfgang Grandegger,
Marc Kleine-Budde, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, linux-can, netdev,
devicetree, Jiri Novak, Oliver Hartkopp
In-Reply-To: <202208020937.54675.pisa@cmp.felk.cvut.cz>
Hi Pavel,
On Tue. 2 Aug. 2022 at 16:38, Pavel Pisa <pisa@cmp.felk.cvut.cz> wrote:
> Hello Vincent,
>
> thanks much for review. I am adding some notices to Tx timestamps
> after your comments
>
> On Tuesday 02 of August 2022 05:43:38 Vincent Mailhol wrote:
> > I just send a series last week which a significant amount of changes
> > for CAN timestamping tree-wide:
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/comm
> >it/?id=12a18d79dc14c80b358dbd26461614b97f2ea4a6
> >
> > I suggest you have a look at this series and harmonize it with the new
> > features (e.g. Hardware TX timestamp).
> >
> > On Tue. 2 Aug. 2022 at 03:52, Matej Vasilevski
> ...
> > > +static int ctucan_hwtstamp_set(struct net_device *dev, struct ifreq
> > > *ifr) +{
> > > + struct ctucan_priv *priv = netdev_priv(dev);
> > > + struct hwtstamp_config cfg;
> > > +
> > > + if (!priv->timestamp_possible)
> > > + return -EOPNOTSUPP;
> > > +
> > > + if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
> > > + return -EFAULT;
> > > +
> > > + if (cfg.flags)
> > > + return -EINVAL;
> > > +
> > > + if (cfg.tx_type != HWTSTAMP_TX_OFF)
> > > + return -ERANGE;
> >
> > I have a great news: your driver now also support hardware TX timestamps:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/comm
> >it/?id=8bdd1112edcd3edce2843e03826204a84a61042d
> >
> > > +
> > > + switch (cfg.rx_filter) {
> > > + case HWTSTAMP_FILTER_NONE:
> > > + priv->timestamp_enabled = false;
> ...
> > > +
> > > + cfg.flags = 0;
> > > + cfg.tx_type = HWTSTAMP_TX_OFF;
> >
> > Hardware TX timestamps are now supported (c.f. supra).
> >
> > > + cfg.rx_filter = priv->timestamp_enabled ? HWTSTAMP_FILTER_ALL :
> > > HWTSTAMP_FILTER_NONE; + return copy_to_user(ifr->ifr_data, &cfg,
> > > sizeof(cfg)) ? -EFAULT : 0; +}
> > > +
> > > +static int ctucan_ioctl(struct net_device *dev, struct ifreq *ifr, int
> > > cmd)
> >
> > Please consider using the generic function can_eth_ioctl_hwts()
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/comm
> >it/?id=90f942c5a6d775bad1be33ba214755314105da4a
> >
> > > +{
> ...
> > > + info->so_timestamping |= SOF_TIMESTAMPING_RX_HARDWARE |
> > > + SOF_TIMESTAMPING_RAW_HARDWARE;
> > > + info->tx_types = BIT(HWTSTAMP_TX_OFF);
> >
> > Hardware TX timestamps are now supported (c.f. supra).
> >
> > > + info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
> > > + BIT(HWTSTAMP_FILTER_ALL);
>
>
> I am not sure if it is good idea to report support for hardware
> TX timestamps by all drivers. Precise hardware Tx timestamps
> are important for some CAN applications but they require to be
> exactly/properly aligned with Rx timestamps.
>
> Only some CAN (FD) controllers really support that feature.
> For M-CAN and some others it is realized as another event
> FIFO in addition to Tx and Rx FIFOs.
>
> For CTU CAN FD, we have decided that we do not complicate design
> and driver by separate events channel. We have configurable
> and possibly large Rx FIFO depth which is logical to use for
> analyzer mode and we can use loopback to receive own messages
> timestamped same way as external received ones.
>
> See 2.14.1 Loopback mode
> SETTINGS[ILBP]=1.
>
> in the datasheet
>
> http://canbus.pages.fel.cvut.cz/ctucanfd_ip_core/doc/Datasheet.pdf
>
> There is still missing information which frames are received
> locally and from which buffer they are in the Rx message format,
> but we plan to add that into VHDL design.
>
> In such case, we can switch driver mode and release Tx buffers
> only after corresponding message is read from Rx FIFO and
> fill exact finegrain (10 ns in our current design) timestamps
> to the echo skb. The order of received messages will be seen
> exactly mathing the wire order for both transmitted and received
> messages then. Which I consider as proper solution for the
> most applications including CAN bus analyzers.
>
> So I consider to report HW Tx timestamps for cases where exact,
> precise timestamping is not available for loopback messages
> as problematic because you cannot distinguish if you talk
> with driver and HW with real/precise timestamps support
> or only dummy implementation to make some tools happy.
Thank you for the explanation. I did not know about the nuance about
those different hardware timestamps.
So if I understand correctly, your hardware can deliver two types of
hardware timestamps:
- The "real" one: fine grained with 10 ns precision when the frame
is actually sent
- The "dummy" one: less precise timestamp generated when there is an
event on the device’s Rx or Tx FIFO.
Is this correct?
Are the "real" and the "dummy" timestamps synced on the same quartz?
What is the precision of the "dummy" timestamp? If it in the order of
magnitude of 10µs? For many use cases, this is enough. 10µs represents
roughly a dozen of time quata (more or less depending on the bitrate
and its prescaler).
Actually, I never saw hardware with a timestamp precision below 1µs
(not saying those don't exist, just that I never encountered them).
I am not against what you propose. But my suggestion would be rather
to report both TX and RX timestamps and just document the precision
(i.e. TX has precision with an order of magnitude of 10µs and RX has
precision of 10 ns).
At the end, I let you decide what works the best for you. Just keep in
mind that the micro second precision is already a great achievement
and not many people need the 10 nano second (especially for CAN).
P.S.: I am on holiday, my answers might be delayed :)
Yours sincerely,
Vincent Mailhol
^ permalink raw reply
* Re: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property
From: Russell King (Oracle) @ 2022-08-12 14:34 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Wei Fang, andrew@lunn.ch, hkallweit1@gmail.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <01021dde-106c-5660-ea96-a8b8fd89ad50@linaro.org>
On Fri, Aug 12, 2022 at 03:36:43PM +0300, Krzysztof Kozlowski wrote:
> On 12/08/2022 15:30, Russell King (Oracle) wrote:
> > On Fri, Aug 12, 2022 at 03:04:41PM +0300, Krzysztof Kozlowski wrote:
> >> I did not propose a property to enable hibernation. The property must
> >> describe hardware, so this piece is missing, regardless whether the
> >> logic in the driver is "enable" or "disable".
> >>
> >> The hardware property for example is: "broken foo, so hibernation should
> >> be disabled" or "engineer forgot to wire cables, so hibernation won't
> >> work"...
> >
> > From the problem description, the PHY itself isn't broken. The stmmac
> > hardware doesn't reset properly when the clock from the PHY is stopped.
>
> There is nothing like that in the property name or property description.
> Again - DT is not for describing driver behavior or driver policy.
Where have I said that it's describing driver behaviour or policy?
Hint: I haven't.
I have no idea why DT maintainers like to keep shoving that idiotic
statement down people's thoats. WE KNOW THIS. And that is NOT what
is being proposed here.
It is purely in your mind that this is a driver behaviour or driver
policy property. It *isn't*.
> > That could hardly be described as "broken" - it's quite common for
> > hardware specifications to state that clocks must be running for the
> > hardware to operate correctly.
> >
> > This is a matter of configuring the hardware to inter-operate correctly.
> > Isn't that the whole purpose of DT?
> >
> > So, nothing is broken. Nothing has forgotten to be wired. It's a matter
> > of properly configuring the hardware. Just the same as selecting the
> > correct interface mode to connect two devices together.
>
> I just gave you two examples what could be written, don't need to stick
> them. You can use some real one...
So the original proposed property to _disable_ a feature implemented by
the hardware should be fine, because it's describing how the hardware
needs to be configured. It's not driver behaviour. It's not driver
policy. It's a configuration bit in a register.
I think you're thinking that the "hibernation" described here is
somehow related to system hibernation, which this has nothing to do
with. This is about configuring the PHY hardware to operate with the
MAC hardware.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v2] wifi: cfg80211: Fix UAF in ieee80211_scan_rx()
From: Siddh Raman Pant @ 2022-08-12 14:33 UTC (permalink / raw)
To: Greg KH
Cc: johannes berg, david s. miller, eric dumazet, jakub kicinski,
paolo abeni, netdev, syzbot+6cb476b7c69916a0caca, linux-wireless,
linux-kernel, syzbot+f9acff9bf08a845f225d,
syzbot+9250865a55539d384347, linux-kernel-mentees
In-Reply-To: <YvZEfnjGIpH6XjsD@kroah.com>
On Fri, 12 Aug 2022 17:45:58 +0530 Greg KH wrote:
> The merge window is for new features to be added, bugfixes can be merged
> at any point in time, but most maintainers close their trees until after
> the merge window is closed before accepting new fixes, like this one.
>
> So just relax, wait another week or so, and if there's no response,
> resend it then.
>
Okay, sure.
> Personally, this patch seems very incorrect, but hey, I'm not the wifi
> subsystem maintainer :)
Why do you think so?
Thanks,
Siddh
^ permalink raw reply
* Re: [PATCH net 1/2] dt: ar803x: Document disable-hibernation property
From: Andrew Lunn @ 2022-08-12 14:15 UTC (permalink / raw)
To: wei.fang
Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, f.fainelli, netdev, devicetree,
linux-kernel
In-Reply-To: <20220812145009.1229094-2-wei.fang@nxp.com>
On Sat, Aug 13, 2022 at 12:50:08AM +1000, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The hibernation mode of Atheros AR803x PHYs is default enabled.
> When the cable is unplugged, the PHY will enter hibernation
> mode and the PHY clock does down. For some MACs, it needs the
> clock to support it's logic. For instance, stmmac needs the PHY
> inputs clock is present for software reset completion. Therefore,
> It is reasonable to add a DT property to disable hibernation mode.
It is not the first time we have seen this. What you should really be
concentrating on is the clock out. That is what the MAC requires here.
You already have the property qca,clk-out-frequency. You could maybe
piggy back off this. If that property is being used, you know the
clock output is used. So you should do what is needed to keep it
ticking.
You also have qca,keep-pll-enabled:
If set, keep the PLL enabled even if there is no link. Useful if you
want to use the clock output without an ethernet link.
To me, it seems like you already have enough properties, you just need
to imply that you need to disable hibernation in order to fulfil these
properties.
Andrew
^ permalink raw reply
* Re: igc: missing HW timestamps at TX
From: Ferenc Fejes @ 2022-08-12 14:13 UTC (permalink / raw)
To: vinicius.gomes@intel.com, netdev@vger.kernel.org
Cc: marton12050@gmail.com, peti.antal99@gmail.com,
vladimir.oltean@nxp.com
In-Reply-To: <87tu6i6h1k.fsf@intel.com>
Hi Vinicius!
On Thu, 2022-08-11 at 10:33 -0300, Vinicius Costa Gomes wrote:
> Hi Ferenc,
>
> >
> > With iperf TCP test line-rate achiveable just like without the
> > patch.
> >
>
> That's very good to know.
>
> > > >
> > > > If you are feeling adventurous and feel like helping test it,
> > > > here
> > > > is
> > > > the link:
> > > >
> > > > https%3A%2F%2Fgithub.com%2Fvcgomes%2Fnet-next%2Ftree%2Figc-
> > > > multiple-tstamp-timers-lock-new
> > > >
> >
> > Is there any test in partucular you interested in? My testbed is
> > configured so I can do some.
> >
>
> The only thing I am worried about is, if in the "dropped" HW
> timestamps
> case, if all the timestamp slots are indeed full, or if there's any
> bug
> and we missed one timestamp.
>
> Can you verify that for for every dropped HW timestamp in your
> application, can you see that 'tx_hwtstamp_skipped' (from 'ethtool -
> S')
> increases everytime the drop happens? Seeing if
> 'tx_hwtstamp_timeouts'
> also increases would be useful as well.
Yes, its increasing. Let me illustrate it:
Ethtool before the measurement:
$ ethtool -S enp3s0 | grep hwtstamp
tx_hwtstamp_timeouts: 1
tx_hwtstamp_skipped: 409
rx_hwtstamp_cleared: 0
Measurement:
$ sudo isochron send -i enp3s0 -s 64 -c 0.0000005 --client 10.0.0.20 --
num-frames 10000000 -F isochron.dat --sync-threshold 2000 -M $((1 <<
2)) --sched-fifo --sched-priority 99
(note: isochron would try to send a packet in every 500ns, but the rate
actually limited by the sleep/syscall latency so its sending packets in
about every 15-20us)
Output:
isochron[1660315948.335677744]: local ptpmon -7 sysmon -
25 receiver ptpmon 0 sysmon 4
Timed out waiting for TX timestamps, 10 timestamps unacknowledged
seqid 3441 missing timestamps: hw,
seqid 3442 missing timestamps: hw,
seqid 3443 missing timestamps: hw,
seqid 3449 missing timestamps: hw,
seqid 5530 missing timestamps: hw,
seqid 5531 missing timestamps: hw,
seqid 7597 missing timestamps: hw,
seqid 7598 missing timestamps: hw,
seqid 7599 missing timestamps: hw,
seqid 7605 missing timestamps: hw,
Ethtool after the measurement:
ethtool -S enp3s0 | grep hwtstamp
tx_hwtstamp_timeouts: 1
tx_hwtstamp_skipped: 419
rx_hwtstamp_cleared: 0
Which is inline with what the isochron see.
But thats only happens if I forcingly put the affinity of the sender
different CPU core than the ptp worker of the igc. If those running on
the same core I doesnt lost any HW timestam even for 10 million
packets. Worth to mention actually I see many lost timestamp which
confused me a little bit but those are lost because of the small
MSG_ERRQUEUE. When I increased that from few kbytes to 20 mbytes I got
every timestamp successfully.
>
> If for every drop there's one 'tx_hwtstamp_skipped' increment, then
> it
> means that the driver is doing its best, and the workload is
> requesting
> more timestamps than the system is able to handle.
>
> If only 'tx_hwtstamp_timeouts' increases then it's possible that
> there
> could be a bug hiding still.
On the other hand I'm little bit confused with the ETF behavior.
Without HW offload, I lost almost every timestamp even with large (one
packet in every 500 us) sending rate and with HW offload I still lost a
lot. But that migh be beyond the igc, and some config issue on my setup
(I have to apply mqprio and do the PTP sync on default priority and
data packets with SO_TXTIME cmsg sent to ETF at prio 2). Does the
tx_queue affect the timestamping?
CC Vladimir, the author of isochron.
>
> > > >
> > > > Cheers,
> > >
> > > Best,
> > > Ferenc
Best,
Ferenc
^ 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