* Re: [PATCH 03/12] net: mana: Handle vport sharing between devices
From: Stephen Hemminger @ 2022-05-17 15:19 UTC (permalink / raw)
To: longli
Cc: longli, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
Wei Liu, Dexuan Cui, David S. Miller, Jakub Kicinski, Paolo Abeni,
Jason Gunthorpe, Leon Romanovsky, linux-hyperv, netdev,
linux-kernel, linux-rdma
In-Reply-To: <1652778276-2986-4-git-send-email-longli@linuxonhyperv.com>
On Tue, 17 May 2022 02:04:27 -0700
longli@linuxonhyperv.com wrote:
> diff --git a/drivers/net/ethernet/microsoft/mana/mana.h b/drivers/net/ethernet/microsoft/mana/mana.h
> index 51bff91b63ee..26f14fcb6a61 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana.h
> +++ b/drivers/net/ethernet/microsoft/mana/mana.h
> @@ -375,6 +375,7 @@ struct mana_port_context {
> unsigned int num_queues;
>
> mana_handle_t port_handle;
> + atomic_t port_use_count;
Could this be a refcount_t instead?
The refcount_t has protections against under/overflow.
^ permalink raw reply
* Re: [RFC PATCH v1 0/8] virtio/vsock: experimental zerocopy receive
From: Stefano Garzarella @ 2022-05-17 15:14 UTC (permalink / raw)
To: Arseniy Krasnov
Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller,
Jakub Kicinski, Paolo Abeni, linux-kernel@vger.kernel.org,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
netdev@vger.kernel.org, kernel
In-Reply-To: <7cdcb1e1-7c97-c054-19cf-5caeacae981d@sberdevices.ru>
Hi Arseniy,
On Thu, May 12, 2022 at 05:04:11AM +0000, Arseniy Krasnov wrote:
> INTRODUCTION
>
> Hello, this is experimental implementation of virtio vsock zerocopy
>receive. It was inspired by TCP zerocopy receive by Eric Dumazet. This API uses
>same idea: call 'mmap()' on socket's descriptor, then every 'getsockopt()' will
>fill provided vma area with pages of virtio RX buffers. After received data was
>processed by user, pages must be freed by 'madvise()' call with MADV_DONTNEED
>flag set(if user won't call 'madvise()', next 'getsockopt()' will fail).
Sounds cool, but maybe we would need some socket/net experts here for
review.
Could we do something similar for the sending path as well?
>
> DETAILS
>
> Here is how mapping with mapped pages looks exactly: first page mapping
>contains array of trimmed virtio vsock packet headers (in contains only length
>of data on the corresponding page and 'flags' field):
>
> struct virtio_vsock_usr_hdr {
> uint32_t length;
> uint32_t flags;
> };
>
>Field 'length' allows user to know exact size of payload within each sequence
>of pages and 'flags' allows user to handle SOCK_SEQPACKET flags(such as message
>bounds or record bounds). All other pages are data pages from RX queue.
>
> Page 0 Page 1 Page N
>
> [ hdr1 .. hdrN ][ data ] .. [ data ]
> | | ^ ^
> | | | |
> | *-------------------*
> | |
> | |
> *----------------*
>
> Of course, single header could represent array of pages (when packet's
>buffer is bigger than one page).So here is example of detailed mapping layout
>for some set of packages. Lets consider that we have the following sequence of
>packages: 56 bytes, 4096 bytes and 8200 bytes. All pages: 0,1,2,3,4 and 5 will
>be inserted to user's vma(vma is large enough).
>
> Page 0: [[ hdr0 ][ hdr 1 ][ hdr 2 ][ hdr 3 ] ... ]
> Page 1: [ 56 ]
> Page 2: [ 4096 ]
> Page 3: [ 4096 ]
> Page 4: [ 4096 ]
> Page 5: [ 8 ]
>
> Page 0 contains only array of headers:
> 'hdr0' has 56 in length field.
> 'hdr1' has 4096 in length field.
> 'hdr2' has 8200 in length field.
> 'hdr3' has 0 in length field(this is end of data marker).
>
> Page 1 corresponds to 'hdr0' and has only 56 bytes of data.
> Page 2 corresponds to 'hdr1' and filled with data.
> Page 3 corresponds to 'hdr2' and filled with data.
> Page 4 corresponds to 'hdr2' and filled with data.
> Page 5 corresponds to 'hdr2' and has only 8 bytes of data.
>
> This patchset also changes packets allocation way: today implementation
>uses only 'kmalloc()' to create data buffer. Problem happens when we try to map
>such buffers to user's vma - kernel forbids to map slab pages to user's vma(as
>pages of "not large" 'kmalloc()' allocations are marked with PageSlab flag and
>"not large" could be bigger than one page). So to avoid this, data buffers now
>allocated using 'alloc_pages()' call.
>
> TESTS
>
> This patchset updates 'vsock_test' utility: two tests for new feature
>were added. First test covers invalid cases. Second checks valid transmission
>case.
Thanks for adding the test!
>
> BENCHMARKING
>
> For benchmakring I've added small utility 'rx_zerocopy'. It works in
>client/server mode. When client connects to server, server starts sending exact
>amount of data to client(amount is set as input argument).Client reads data and
>waits for next portion of it. Client works in two modes: copy and zero-copy. In
>copy mode client uses 'read()' call while in zerocopy mode sequence of 'mmap()'
>/'getsockopt()'/'madvise()' are used. Smaller amount of time for transmission
>is better. For server, we can set size of tx buffer and for client we can set
>size of rx buffer or rx mapping size(in zerocopy mode). Usage of this utility
>is quiet simple:
>
>For client mode:
>
>./rx_zerocopy --mode client [--zerocopy] [--rx]
>
>For server mode:
>
>./rx_zerocopy --mode server [--mb] [--tx]
>
>[--mb] sets number of megabytes to transfer.
>[--rx] sets size of receive buffer/mapping in pages.
>[--tx] sets size of transmit buffer in pages.
>
>I checked for transmission of 4000mb of data. Here are some results:
>
> size of rx/tx buffers in pages
> *---------------------------------------------------*
> | 8 | 32 | 64 | 256 | 512 |
>*--------------*--------*----------*---------*----------*----------*
>| zerocopy | 24 | 10.6 | 12.2 | 23.6 | 21 | secs to
>*--------------*---------------------------------------------------- process
>| non-zerocopy | 13 | 16.4 | 24.7 | 27.2 | 23.9 | 4000 mb
>*--------------*----------------------------------------------------
>
>I think, that results are not so impressive, but at least it is not worse than
>copy mode and there is no need to allocate memory for processing date.
Why is it twice as slow in the first column?
>
> PROBLEMS
>
> Updated packet's allocation logic creates some problem: when host gets
>data from guest(in vhost-vsock), it allocates at least one page for each packet
>(even if packet has 1 byte payload). I think this could be resolved in several
>ways:
Can we somehow copy the incoming packets into the payload of the already
queued packet?
This reminds me that we have yet to fix a similar problem with kmalloc()
as well...
https://bugzilla.kernel.org/show_bug.cgi?id=215329
> 1) Make zerocopy rx mode disabled by default, so if user didn't enable
>it, current 'kmalloc()' way will be used.
That sounds reasonable to me, I guess also TCP needs a setsockopt() call
to enable the feature, right?
> 2) Use 'kmalloc()' for "small" packets, else call page allocator. But
>in this case, we have mix of packets, allocated in two different ways thus
>during zerocopying to user(e.g. mapping pages to vma), such small packets will
>be handled in some stupid way: we need to allocate one page for user, copy data
>to it and then insert page to user's vma.
It seems more difficult to me, but at the same time doable. I would go
more on option 1, though.
>
>P.S: of course this is experimental RFC, so what do You think guys?
It seems cool :-)
But I would like some feedback from the net guys to have some TCP-like
things.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: net: marvell,orion-mdio: Set unevaluatedProperties to false
From: Rob Herring @ 2022-05-17 15:11 UTC (permalink / raw)
To: Chris Packham
Cc: sebastian.hesselbarth, robh+dt, davem, pabeni, andrew, edumazet,
kuba, kabel, krzysztof.kozlowski+dt, devicetree, linux-arm-kernel,
netdev, linux-kernel, gregory.clement
In-Reply-To: <20220516224801.1656752-3-chris.packham@alliedtelesis.co.nz>
On Tue, 17 May 2022 10:48:01 +1200, Chris Packham wrote:
> When the binding was converted it appeared necessary to set
> 'unevaluatedProperties: true' because of the switch devices on the
> turris-mox board. Actually the error was because of the reg property
> being incorrect causing the rest of the properties to be unevaluated.
>
> After the reg properties are fixed for turris-mox we can set
> 'unevaluatedProperties: false' as is generally expected.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Documentation/devicetree/bindings/net/marvell,orion-mdio.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 09/12] net: mana: Move header files to a common location
From: Jason Gunthorpe @ 2022-05-17 15:03 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, David S. Miller, Jakub Kicinski, Paolo Abeni,
Leon Romanovsky, linux-hyperv, netdev, linux-kernel, linux-rdma
In-Reply-To: <1652778276-2986-10-git-send-email-longli@linuxonhyperv.com>
On Tue, May 17, 2022 at 02:04:33AM -0700, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> In preparation to add MANA RDMA driver, move all the required header files
> to a common location for use by both Ethernet and RDMA drivers.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> MAINTAINERS | 1 +
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 2 +-
> drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 ++--
> drivers/net/ethernet/microsoft/mana/mana_bpf.c | 2 +-
> drivers/net/ethernet/microsoft/mana/mana_en.c | 2 +-
> drivers/net/ethernet/microsoft/mana/mana_ethtool.c | 2 +-
> drivers/net/ethernet/microsoft/mana/shm_channel.c | 2 +-
> {drivers/net/ethernet/microsoft => include/linux}/mana/gdma.h | 0
> .../ethernet/microsoft => include/linux}/mana/hw_channel.h | 0
> {drivers/net/ethernet/microsoft => include/linux}/mana/mana.h | 0
> .../ethernet/microsoft => include/linux}/mana/shm_channel.h | 0
> 11 files changed, 8 insertions(+), 7 deletions(-)
> rename {drivers/net/ethernet/microsoft => include/linux}/mana/gdma.h (100%)
> rename {drivers/net/ethernet/microsoft => include/linux}/mana/hw_channel.h (100%)
> rename {drivers/net/ethernet/microsoft => include/linux}/mana/mana.h (100%)
> rename {drivers/net/ethernet/microsoft => include/linux}/mana/shm_channel.h (100%)
I know mlx5 did it like this, but I wonder if include/net is more
appropriate?
Or maybe include/aux/?
Jason
^ permalink raw reply
* Re: [PATCH 05/12] net: mana: Set the DMA device max page size
From: Jason Gunthorpe @ 2022-05-17 14:59 UTC (permalink / raw)
To: longli
Cc: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, David S. Miller, Jakub Kicinski, Paolo Abeni,
Leon Romanovsky, linux-hyperv, netdev, linux-kernel, linux-rdma
In-Reply-To: <1652778276-2986-6-git-send-email-longli@linuxonhyperv.com>
On Tue, May 17, 2022 at 02:04:29AM -0700, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> The system chooses default 64K page size if the device does not specify
> the max page size the device can handle for DMA. This do not work well
> when device is registering large chunk of memory in that a large page size
> is more efficient.
>
> Set it to the maximum hardware supported page size.
For RDMA devices this should be set to the largest segment size an
ib_sge can take in when posting work. It should not be the page size
of MR. 2M is a weird number for that, are you sure it is right?
Jason
^ permalink raw reply
* Re: [PATCH wpan-next v2 10/11] net: mac802154: Add a warning in the hot path
From: Miquel Raynal @ 2022-05-17 14:52 UTC (permalink / raw)
To: Alexander Aring
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, Network Development, David Girault,
Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <20220517153655.155ba311@xps-13>
miquel.raynal@bootlin.com wrote on Tue, 17 May 2022 15:36:55 +0200:
> aahringo@redhat.com wrote on Sun, 15 May 2022 18:30:15 -0400:
>
> > Hi,
> >
> > On Thu, May 12, 2022 at 10:34 AM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > >
> > > We should never start a transmission after the queue has been stopped.
> > >
> > > But because it might work we don't kill the function here but rather
> > > warn loudly the user that something is wrong.
> > >
> > > Set an atomic when the queue will remain stopped. Reset this atomic when
> > > the queue actually gets restarded. Just check this atomic to know if the
> > > transmission is legitimate, warn if it is not.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > > include/net/cfg802154.h | 1 +
> > > net/mac802154/tx.c | 16 +++++++++++++++-
> > > net/mac802154/util.c | 1 +
> > > 3 files changed, 17 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > index 8b6326aa2d42..a1370e87233e 100644
> > > --- a/include/net/cfg802154.h
> > > +++ b/include/net/cfg802154.h
> > > @@ -218,6 +218,7 @@ struct wpan_phy {
> > > struct mutex queue_lock;
> > > atomic_t ongoing_txs;
> > > atomic_t hold_txs;
> > > + atomic_t queue_stopped;
> >
> > Maybe some test_bit()/set_bit() is better there?
>
> What do you mean? Shall I change the atomic_t type of queue_stopped?
> Isn't the atomic_t preferred in this situation?
Actually I re-read the doc and that's right, a regular unsigned long
used with test/set_bit might be preferred, I'll make the change.
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add selftest for bpf_ct_refresh_timeout kfunc
From: Lorenzo Bianconi @ 2022-05-17 14:42 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, David S. Miller, Jakub Kicinski, Paolo Abeni,
Pablo Neira Ayuso, Florian Westphal, netfilter-devel,
Lorenzo Bianconi, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Kumar Kartikeya Dwivedi
In-Reply-To: <CAADnVQJbOZAg-nGrVutwCA5r=VATXVOXD5Y2EtbfkZHtCsrBbg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]
> On Sat, May 14, 2022 at 3:40 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > > On Thu, May 12, 2022 at 9:34 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > > >
> > > > Install a new ct entry in order to perform a successful lookup and
> > > > test bpf_ct_refresh_timeout kfunc helper.
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > >
> > > CI is failing:
> > > test_bpf_nf_ct:FAIL:flush ct entries unexpected error: 32512 (errno 2)
> > > test_bpf_nf_ct:FAIL:create ct entry unexpected error: 32512 (errno 2)
> > >
> > > Please follow the links from patchwork for details.
> >
> > Hi Alexei,
> >
> > tests failed because conntrack is not installed on the system:
> >
> > 2022-05-14T00:12:09.0799053Z sh: line 1: conntrack: command not found
> >
> > Is it ok to just skip the test if conntrack is not installed on the system
> > or do you prefer to directly send netlink messages to ct in order to add a
> > new ct entry?
>
> It will take a long time to update x86 and s390 images.
> Maybe we should add a kfunc that creates a ct entry?
ack, I added the support for it. I will post it soon.
Regards,
Lorenzo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 net-next 02/15] dt-bindings: net: mediatek,net: add mt7986-eth binding
From: Rob Herring @ 2022-05-17 14:36 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: davem, linux-mediatek, Sam.Shih, pabeni, lorenzo.bianconi,
Mark-MC.Lee, kuba, nbd, sean.wang, edumazet, netdev, john,
devicetree
In-Reply-To: <aa934c3185c9e04893d9c285ed655495a049fa4f.1652716741.git.lorenzo@kernel.org>
On Mon, 16 May 2022 18:06:29 +0200, Lorenzo Bianconi wrote:
> Introduce dts bindings for mt7986 soc in mediatek,net.yaml.
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../devicetree/bindings/net/mediatek,net.yaml | 141 +++++++++++++++++-
> 1 file changed, 139 insertions(+), 2 deletions(-)
>
Doesn't apply for me, so not tested, but:
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Oliver Hartkopp @ 2022-05-17 14:35 UTC (permalink / raw)
To: Max Staudt
Cc: Marc Kleine-Budde, Vincent MAILHOL, linux-can, linux-kernel,
netdev
In-Reply-To: <20220517154301.5bf99ba9.max@enpas.org>
On 17.05.22 15:43, Max Staudt wrote:
> On Tue, 17 May 2022 15:35:03 +0200
> Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
>> Oh, I didn't want to introduce two new kernel modules but to have
>> can_dev in different 'feature levels'.
>
> Which I agree is a nice idea, as long as heisenbugs can be avoided :)
>
> (as for the separate modules vs. feature levels of can-dev - sorry, my
> two paragraphs were each referring to a different idea. I mixed them
> into one single email...)
>
>
> Maybe the can-skb and rx-offload parts could be a *visible* sub-option
> of can-dev in Kconfig, which is normally optional, but immediately
> force-selected once a CAN HW driver is selected?
I think it should be even more simple.
When you enter the current Kconfig page of "CAN Device Drivers" every
selection of vcan/vxcan/slcan should directly select CAN_DEV_SW.
The rest could stay the same, e.g. selecting CAN_DEV "Platform CAN
drivers with Netlink support" which then enables CAN_CALC_BITTIMING and
CAN_LEDS to be selectable. Which also makes sure the old .config files
still apply.
And finally the selection of flexcan, ti_hecc and
mcp251xfd automatically selects CAN_DEV_RX_OFFLOAD.
Then only some more Makefile magic has to be done to build can-dev.ko
accordingly.
Best regards,
Oliver
>
>
>> But e.g. the people that are running Linux instances in a cloud only
>> using vcan and vxcan would not need to carry the entire
>> infrastructure of CAN hardware support and rx-offload.
>
> Out of curiosity, do you have an example use case for this vcan cloud
> setup? I can't dream one up...
>
>
>
> Max
^ permalink raw reply
* Re: Bonding problem on Intel X710 hardware
From: Sven Anders @ 2022-05-17 14:23 UTC (permalink / raw)
To: netdev
In-Reply-To: <700118d5-2007-3c13-af2d-3a2a6c7775bd@anduras.de>
Hello!
This is a follow up to my question. I did not hear anything so far, but I tried
to find some some information meanwhile.
I've got a guess from somebody, that the error message "Error I40E_AQ_RC_EINVAL
adding RX filters on PF, promiscuous mode forced on" maybe triggered, because
I'm hitting a limit here.
Somebody other said, that this seems to be an error in the "bonding driver", but
I do not think so. Aside from that, there seem to be no special "bonding" mailing
list anymore. So I will have to ask this questions here anyway...
I want to understand the problem to classify it.
1) Why is this "error" issued? Do I really hit a limit and what is this current limit?
2) Is it really an error or is it more "a warning"?
3) Why is this error triggered only when changing the "ntuples filter" and not during
the normal adding of VLANs?
Remark: I can trigger the "ntuples fiilter" later on again and it still works.
I also tried the latest 5.18-rc kernel with the same problem.
Maybe somebody will find time and try to reproduce this?
I will answer any questions...
Regards
Sven
Am 12.05.22 um 16:05 schrieb Sven Anders:
> Hello!
>
> I'm having problems setting up a bond in adaptive load balancing
> mode (balance-alb, mode 6) on an intel X710 network adapter using
> the i40e driver connected to an Aruba 2530-48G switch.
> The network card has 4 on board ports.
> I'm using 2 ports for the bond with 36 VLANs on it.
>
> The setup is correct, because it works without problems, if
> I use the same setup with 1GBit Intel hardware (using the
> e1000e driver, version 3.2.6-k, firmware 5.10-2).
>
> Data packets are only received sporadically. If I run the same test
> with only one slave port, it works without problems.
>
> I debugged it down to the reception of the packets by the
> network hardware.
>
> If I remove the number of VLANs under 8, almost all packets are
> received. The fewer VLANs the better the receive rate.
>
> I suspected the hardware offloading operations to be the cause, so I
> tried to disable them. It resulted in the following:
>
> If I turn of the "ntuple-filters" with
> ethtool -K eth3 ntuple off
> ethtool -K eth3 ntuple off
> it will work.
>
> But if I do this I see the following errors in "dmesg":
> i40e 0000:65:00.1: Error I40E_AQ_RC_EINVAL adding RX filters on PF, promiscuous mode forced on
> i40e 0000:65:00.2: Error I40E_AQ_RC_EINVAL adding RX filters on PF, promiscuous mode forced on
>
> Disabling any any other offloading operations made no change.
>
> For me it seems, that the hardware filter is dropping packets because they
> have the wrong values (mac-address ?).
> Turning the "ntuple-filters" off, forces the network adapter to accept
> all packets.
>
>
> My questions:
>
> 1. Can anybody explain or confirm this?
>
> 2. Is the a correct method to force the adapter in promiscous mode?
>
> 3. Are the any special settings needed, if I use ALB bonding, which I missed?
>
>
> Some details:
> -------------
>
> Linux kernel 5.15.35-core2 on x86_64.
>
>
> This is the hardware:
> ---------------------
> 4 port Ethernet controller:
> Intel Corporation Ethernet Controller X710 for 10GBASE-T (rev 02)
> 8086:15ff (rev 02)
>
> with
>
> driver: i40e
> version: 5.15.35-core2
> firmware-version: 8.60 0x8000bd80 1.3140.0
> bus-info: 0000:65:00.2
> supports-statistics: yes
> supports-test: yes
> supports-eeprom-access: yes
> supports-register-dump: yes
> supports-priv-flags: yes
>
>
> This is current bonding configuration:
> --------------------------------------
> Ethernet Channel Bonding Driver: v5.15.35-core2
>
> Bonding Mode: adaptive load balancing
> Primary Slave: None
> Currently Active Slave: eth3
> MII Status: up
> MII Polling Interval (ms): 100
> Up Delay (ms): 200
> Down Delay (ms): 200
> Peer Notification Delay (ms): 0
>
> Slave Interface: eth3
> MII Status: up
> Speed: 1000 Mbps
> Duplex: full
> Link Failure Count: 0
> Permanent HW addr: 68:05:ca:f8:9c:42
> Slave queue ID: 0
>
> Slave Interface: eth4
> MII Status: up
> Speed: 1000 Mbps
> Duplex: full
> Link Failure Count: 0
> Permanent HW addr: 68:05:ca:f8:9c:41
> Slave queue ID: 0
>
>
> Regards
> Sven Anders
>
Mit freundlichen Grüßen
Sven Anders
--
Sven Anders <anders@anduras.de> () UTF-8 Ribbon Campaign
/\ Support plain text e-mail
ANDURAS intranet security AG
Messestraße 3 - 94036 Passau - Germany
Web: www.anduras.de - Tel: +49 (0)851-4 90 50-0
Rechtsform: Aktiengesellschaft - Sitz: Passau - Amtsgericht: Passau HRB 6032
Vorstand: Marcus Junker
Vorsitzender des Aufsichtsrats: Andreas Wagner
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Marc Kleine-Budde @ 2022-05-17 14:23 UTC (permalink / raw)
To: Max Staudt
Cc: Oliver Hartkopp, Vincent MAILHOL, linux-can, linux-kernel, netdev
In-Reply-To: <20220517154301.5bf99ba9.max@enpas.org>
[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]
On 17.05.2022 15:43:01, Max Staudt wrote:
> > Oh, I didn't want to introduce two new kernel modules but to have
> > can_dev in different 'feature levels'.
>
> Which I agree is a nice idea, as long as heisenbugs can be avoided :)
>
> (as for the separate modules vs. feature levels of can-dev - sorry, my
> two paragraphs were each referring to a different idea. I mixed them
> into one single email...)
>
> Maybe the can-skb and rx-offload parts could be a *visible* sub-option
> of can-dev in Kconfig, which is normally optional, but immediately
> force-selected once a CAN HW driver is selected?
In the ctucanfd driver we made the base driver "invisible" if
COMPILE_TEST is not selected:
| config CAN_CTUCANFD
| tristate "CTU CAN-FD IP core" if COMPILE_TEST
|
| config CAN_CTUCANFD_PCI
| tristate "CTU CAN-FD IP core PCI/PCIe driver"
| depends on PCI
| select CAN_CTUCANFD
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] gcc: fix -Warray-compare
From: Martin Liška @ 2022-05-17 14:19 UTC (permalink / raw)
To: netdev, lkml; +Cc: davem, edumazet, kuba, pabeni
Fixes the following GCC warning:
drivers/net/ethernet/sun/cassini.c:1316:29: error: comparison between two arrays [-Werror=array-compare]
drivers/net/ethernet/sun/cassini.c:3783:34: error: comparison between two arrays [-Werror=array-compare]
Signed-off-by: Martin Liska <mliska@suse.cz>
---
drivers/net/ethernet/sun/cassini.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index b04a6a7bf566..435dc00d04e5 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1313,7 +1313,7 @@ static void cas_init_rx_dma(struct cas *cp)
writel(val, cp->regs + REG_RX_PAGE_SIZE);
/* enable the header parser if desired */
- if (CAS_HP_FIRMWARE == cas_prog_null)
+ if (&CAS_HP_FIRMWARE[0] == &cas_prog_null[0])
return;
val = CAS_BASE(HP_CFG_NUM_CPU, CAS_NCPUS > 63 ? 0 : CAS_NCPUS);
@@ -3780,7 +3780,7 @@ static void cas_reset(struct cas *cp, int blkflag)
/* program header parser */
if ((cp->cas_flags & CAS_FLAG_TARGET_ABORT) ||
- (CAS_HP_ALT_FIRMWARE == cas_prog_null)) {
+ (&CAS_HP_ALT_FIRMWARE[0] == &cas_prog_null[0])) {
cas_load_firmware(cp, CAS_HP_FIRMWARE);
} else {
cas_load_firmware(cp, CAS_HP_ALT_FIRMWARE);
--
2.36.1
^ permalink raw reply related
* Re: [PATCH 21/30] panic: Introduce the panic pre-reboot notifier list
From: Petr Mladek @ 2022-05-17 14:11 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: Luck, Tony, Dinh Nguyen, akpm@linux-foundation.org,
bhe@redhat.com, kexec@lists.infradead.org,
linux-kernel@vger.kernel.org,
bcm-kernel-feedback-list@broadcom.com,
linuxppc-dev@lists.ozlabs.org, linux-alpha@vger.kernel.org,
linux-edac@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-leds@vger.kernel.org, linux-mips@vger.kernel.org,
linux-parisc@vger.kernel.org, linux-pm@vger.kernel.org,
linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org,
linux-tegra@vger.kernel.org, linux-um@lists.infradead.org,
linux-xtensa@linux-xtensa.org, netdev@vger.kernel.org,
openipmi-developer@lists.sourceforge.net, rcu@vger.kernel.org,
sparclinux@vger.kernel.org, xen-devel@lists.xenproject.org,
x86@kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net,
halves@canonical.com, fabiomirmar@gmail.com,
alejandro.j.jimenez@oracle.com, andriy.shevchenko@linux.intel.com,
arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
d.hatayama@jp.fujitsu.com, dave.hansen@linux.intel.com,
dyoung@redhat.com, Tang, Feng, gregkh@linuxfoundation.org,
mikelley@microsoft.com, hidehiro.kawai.ez@hitachi.com,
jgross@suse.com, john.ogness@linutronix.de, keescook@chromium.org,
luto@kernel.org, mhiramat@kernel.org, mingo@redhat.com,
paulmck@kernel.org, peterz@infradead.org, rostedt@goodmis.org,
senozhatsky@chromium.org, stern@rowland.harvard.edu,
tglx@linutronix.de, vgoyal@redhat.com, vkuznets@redhat.com,
will@kernel.org, Alex Elder, Alexander Gordeev, Anton Ivanov,
Benjamin Herrenschmidt, Bjorn Andersson, Boris Ostrovsky,
Chris Zankel, Christian Borntraeger, Corey Minyard, Dexuan Cui,
H. Peter Anvin, Haiyang Zhang, Heiko Carstens, Helge Deller,
Ivan Kokshaysky, James E.J. Bottomley, James Morse, Johannes Berg,
K. Y. Srinivasan, Mathieu Poirier, Matt Turner,
Mauro Carvalho Chehab, Max Filippov, Michael Ellerman,
Paul Mackerras, Pavel Machek, Richard Weinberger, Robert Richter,
Stefano Stabellini, Stephen Hemminger, Sven Schnelle,
Vasily Gorbik, Wei Liu
In-Reply-To: <e895ce94-e6b9-caf6-e5d3-06bf0149445c@igalia.com>
On Mon 2022-05-16 13:33:51, Guilherme G. Piccoli wrote:
> On 16/05/2022 13:18, Luck, Tony wrote:
> >> [...]
> > Would it be possible to have some global "kdump is configured + enabled" flag?
> >
> > Then notifiers could make an informed choice on whether to deep dive to
> > get all the possible details (when there is no kdump) or just skim the high
> > level stuff (to maximize chance of getting a successful kdump).
> >
> > -Tony
>
> Good idea Tony! What if I wire a kexec_crash_loaded() in the notifier?
I like this idea.
One small problem is that kexec_crash_loaded() has valid result
only under kexec_mutex. On the other hand, it should stay true
once loaded so that the small race window should be innocent.
> With that, are you/Petr/Dinh OK in moving it for the info list?
Sounds good to me.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
From: Petr Mladek @ 2022-05-17 13:57 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: David Gow, Evan Green, Julius Werner, Scott Branden,
bcm-kernel-feedback-list, Sebastian Reichel, linux-pm,
Florian Fainelli, akpm, bhe, kexec, linux-kernel, linuxppc-dev,
linux-alpha, linux-arm-kernel, linux-edac, linux-hyperv,
linux-leds, linux-mips, linux-parisc, linux-remoteproc,
linux-s390, linux-tegra, linux-um, linux-xtensa, netdev,
openipmi-developer, rcu, sparclinux, xen-devel, x86, kernel-dev,
kernel, halves, fabiomirmar, alejandro.j.jimenez,
andriy.shevchenko, arnd, bp, corbet, d.hatayama, dave.hansen,
dyoung, feng.tang, gregkh, mikelley, hidehiro.kawai.ez, jgross,
john.ogness, keescook, luto, mhiramat, mingo, paulmck, peterz,
rostedt, senozhatsky, stern, tglx, vgoyal, vkuznets, will,
Alexander Gordeev, Andrea Parri, Ard Biesheuvel,
Benjamin Herrenschmidt, Brian Norris, Christian Borntraeger,
Christophe JAILLET, David S. Miller, Dexuan Cui, Doug Berger,
Haiyang Zhang, Hari Bathini, Heiko Carstens, Justin Chen,
K. Y. Srinivasan, Lee Jones, Markus Mayer, Michael Ellerman,
Mihai Carabas, Nicholas Piggin, Paul Mackerras, Pavel Machek,
Shile Zhang, Stephen Hemminger, Sven Schnelle,
Thomas Bogendoerfer, Tianyu Lan, Vasily Gorbik, Wang ShaoBo,
Wei Liu, zhenwei pi
In-Reply-To: <ad082ce7-db50-13bb-3dbb-9b595dfa78be@igalia.com>
On Mon 2022-05-16 12:06:17, Guilherme G. Piccoli wrote:
> Thanks for the review!
>
> I agree with the blinking stuff, I can rework and add all LED/blinking
> stuff into the loop list, it does make sense. I'll comment a bit in the
> others below...
>
> On 16/05/2022 11:01, Petr Mladek wrote:
> >> --- a/drivers/firmware/google/gsmi.c
> >> +++ b/drivers/firmware/google/gsmi.c
> >> @@ -1034,7 +1034,7 @@ static __init int gsmi_init(void)
> >>
> >> register_reboot_notifier(&gsmi_reboot_notifier);
> >> register_die_notifier(&gsmi_die_notifier);
> >> - atomic_notifier_chain_register(&panic_notifier_list,
> >> + atomic_notifier_chain_register(&panic_hypervisor_list,
> >> &gsmi_panic_notifier);
> >
> > I am not sure about this one. It looks like some logging or
> > pre_reboot stuff.
> >
>
> Disagree here. I'm looping Google maintainers, so they can comment.
> (CCed Evan, David, Julius)
>
> This notifier is clearly a hypervisor notification mechanism. I've fixed
> a locking stuff there (in previous patch), I feel it's low-risk but even
> if it's mid-risk, the class of such callback remains a perfect fit with
> the hypervisor list IMHO.
It is similar to drivers/soc/bcm/brcmstb/pm/pm-arm.c.
See below for another idea.
> >> --- a/drivers/misc/bcm-vk/bcm_vk_dev.c
> >> +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c
> >> @@ -1446,7 +1446,7 @@ static int bcm_vk_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>
> >> /* register for panic notifier */
> >> vk->panic_nb.notifier_call = bcm_vk_on_panic;
> >> - err = atomic_notifier_chain_register(&panic_notifier_list,
> >> + err = atomic_notifier_chain_register(&panic_hypervisor_list,
> >> &vk->panic_nb);
> >
> > It seems to reset some hardware or so. IMHO, it should go into the
> > pre-reboot list.
>
> Mixed feelings here, I'm looping Broadcom maintainers to comment.
> (CC Scott and Broadcom list)
>
> I'm afraid it breaks kdump if this device is not reset beforehand - it's
> a doorbell write, so not high risk I think...
>
> But in case the not-reset device can be probed normally in kdump kernel,
> then I'm fine in moving this to the reboot list! I don't have the HW to
> test myself.
Good question. Well, it if has to be called before kdump then
even "hypervisor" list is a wrong place because is not always
called before kdump.
> >> --- a/drivers/power/reset/ltc2952-poweroff.c
> >> +++ b/drivers/power/reset/ltc2952-poweroff.c
> >> @@ -279,7 +279,7 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
> >> pm_power_off = ltc2952_poweroff_kill;
> >>
> >> data->panic_notifier.notifier_call = ltc2952_poweroff_notify_panic;
> >> - atomic_notifier_chain_register(&panic_notifier_list,
> >> + atomic_notifier_chain_register(&panic_hypervisor_list,
> >> &data->panic_notifier);
> >
> > I looks like this somehow triggers the reboot. IMHO, it should go
> > into the pre_reboot list.
>
> Mixed feeling again here - CCing the maintainers for comments (Sebastian
> / PM folks).
>
> This is setting a variable only, and once it's set (data->kernel_panic
> is the bool's name), it just bails out the IRQ handler and a timer
> setting - this timer seems kinda tricky, so bailing out ASAP makes sense
> IMHO.
IMHO, the timer informs the hardware that the system is still alive
in the middle of panic(). If the timer is not working then the
hardware (chip) will think that the system frozen in panic()
and will power off the system. See the comments in
drivers/power/reset/ltc2952-poweroff.c:
* The following GPIOs are used:
* - trigger (input)
* A level change indicates the shut-down trigger. If it's state reverts
* within the time-out defined by trigger_delay, the shut down is not
* executed. If no pin is assigned to this input, the driver will start the
* watchdog toggle immediately. The chip will only power off the system if
* it is requested to do so through the kill line.
*
* - watchdog (output)
* Once a shut down is triggered, the driver will toggle this signal,
* with an internal (wde_interval) to stall the hardware shut down.
IMHO, we really have to keep it alive until we reach the reboot stage.
Another question is how it actually works when the interrupts are
disabled during panic() and the timer callbacks are not handled.
> > [...]
> >> --- a/drivers/soc/bcm/brcmstb/pm/pm-arm.c
> >> +++ b/drivers/soc/bcm/brcmstb/pm/pm-arm.c
> >> @@ -814,7 +814,7 @@ static int brcmstb_pm_probe(struct platform_device *pdev)
> >> goto out;
> >> }
> >>
> >> - atomic_notifier_chain_register(&panic_notifier_list,
> >> + atomic_notifier_chain_register(&panic_hypervisor_list,
> >> &brcmstb_pm_panic_nb);
> >
> > I am not sure about this one. It instruct some HW to preserve DRAM.
> > IMHO, it better fits into pre_reboot category but I do not have
> > strong opinion.
>
> Disagree here, I'm CCing Florian for information.
>
> This notifier preserves RAM so it's *very interesting* if we have
> kmsg_dump() for example, but maybe might be also relevant in case kdump
> kernel is configured to store something in a persistent RAM (then,
> without this notifier, after kdump reboots the system data would be lost).
I see. It is actually similar problem as with
drivers/firmware/google/gsmi.c.
I does similar things like kmsg_dump() so it should be called in
the same location (after info notifier list and before kdump).
A solution might be to put it at these notifiers at the very
end of the "info" list or make extra "dump" notifier list.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 0/3] Macb PTP updates
From: Richard Cochran @ 2022-05-17 13:55 UTC (permalink / raw)
To: Harini Katakam
Cc: nicolas.ferre, davem, claudiu.beznea, kuba, pabeni, netdev,
linux-kernel, michal.simek, harinikatakamlinux,
radhey.shyam.pandey
In-Reply-To: <20220517073259.23476-1-harini.katakam@xilinx.com>
On Tue, May 17, 2022 at 01:02:56PM +0530, Harini Katakam wrote:
> Macb PTP updates to handle PTP one step sync support and other minor
> enhancements.
>
> Harini Katakam (3):
> net: macb: Fix PTP one step sync support
> net: macb: Enable PTP unicast
> net: macb: Optimize reading HW timestamp
>
> drivers/net/ethernet/cadence/macb.h | 4 ++
> drivers/net/ethernet/cadence/macb_main.c | 61 +++++++++++++++++++-----
> drivers/net/ethernet/cadence/macb_ptp.c | 12 +++--
> 3 files changed, 63 insertions(+), 14 deletions(-)
For the series:
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* Re: [PATCH wpan-next v2 11/11] net: mac802154: Add a warning in the slow path
From: Miquel Raynal @ 2022-05-17 13:45 UTC (permalink / raw)
To: Alexander Aring
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, Network Development, David Girault,
Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <CAK-6q+iuB4kFOP7RwwaFQ9AbQTijrmXBzDis7wXo2Pat=cW6kA@mail.gmail.com>
Hi Alexander,
aahringo@redhat.com wrote on Sun, 15 May 2022 18:30:28 -0400:
> Hi,
>
> On Thu, May 12, 2022 at 10:34 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > In order to be able to detect possible conflicts between the net
> > interface core and the ieee802154 core, let's add a warning in the slow
> > path: we want to be sure that whenever we start an asynchronous MLME
> > transmission (which can be fully asynchronous) the net core somehow
> > agrees that this transmission is possible, ie. the device was not
> > stopped. Warning in this case would allow us to track down more easily
> > possible issues with the MLME logic if we ever get reports.
> >
> > Unlike in the hot path, such a situation cannot be handled.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > net/mac802154/tx.c | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
> > index a3c9f194c025..d61b076239c3 100644
> > --- a/net/mac802154/tx.c
> > +++ b/net/mac802154/tx.c
> > @@ -132,6 +132,25 @@ int ieee802154_sync_and_hold_queue(struct ieee802154_local *local)
> > return ret;
> > }
> >
> > +static bool ieee802154_netif_is_down(struct ieee802154_local *local)
> > +{
> > + struct ieee802154_sub_if_data *sdata;
> > + bool is_down = false;
> > +
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> > + if (!sdata->dev)
> > + continue;
> > +
> > + is_down = !(sdata->dev->flags & IFF_UP);
>
> Is there not a helper for this flag?
I was surprised that nobody cared enough about that information to
create a helper. Then I grepped and figured out I was not the first to
to do that...
$ git grep "flags & IFF_UP" | wc -l
289
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Max Staudt @ 2022-05-17 13:43 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Marc Kleine-Budde, Vincent MAILHOL, linux-can, linux-kernel,
netdev
In-Reply-To: <0b505b1f-1ee4-5a2c-3bbf-6e9822f78817@hartkopp.net>
On Tue, 17 May 2022 15:35:03 +0200
Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Oh, I didn't want to introduce two new kernel modules but to have
> can_dev in different 'feature levels'.
Which I agree is a nice idea, as long as heisenbugs can be avoided :)
(as for the separate modules vs. feature levels of can-dev - sorry, my
two paragraphs were each referring to a different idea. I mixed them
into one single email...)
Maybe the can-skb and rx-offload parts could be a *visible* sub-option
of can-dev in Kconfig, which is normally optional, but immediately
force-selected once a CAN HW driver is selected?
> But e.g. the people that are running Linux instances in a cloud only
> using vcan and vxcan would not need to carry the entire
> infrastructure of CAN hardware support and rx-offload.
Out of curiosity, do you have an example use case for this vcan cloud
setup? I can't dream one up...
Max
^ permalink raw reply
* Re: [PATCH wpan-next v2 10/11] net: mac802154: Add a warning in the hot path
From: Miquel Raynal @ 2022-05-17 13:36 UTC (permalink / raw)
To: Alexander Aring
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, Network Development, David Girault,
Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <CAK-6q+jYb7A2RzG3u7PJYKZU9D5A=vben-Wnu-3EsUU-rqGT2Q@mail.gmail.com>
aahringo@redhat.com wrote on Sun, 15 May 2022 18:30:15 -0400:
> Hi,
>
> On Thu, May 12, 2022 at 10:34 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > We should never start a transmission after the queue has been stopped.
> >
> > But because it might work we don't kill the function here but rather
> > warn loudly the user that something is wrong.
> >
> > Set an atomic when the queue will remain stopped. Reset this atomic when
> > the queue actually gets restarded. Just check this atomic to know if the
> > transmission is legitimate, warn if it is not.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > include/net/cfg802154.h | 1 +
> > net/mac802154/tx.c | 16 +++++++++++++++-
> > net/mac802154/util.c | 1 +
> > 3 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > index 8b6326aa2d42..a1370e87233e 100644
> > --- a/include/net/cfg802154.h
> > +++ b/include/net/cfg802154.h
> > @@ -218,6 +218,7 @@ struct wpan_phy {
> > struct mutex queue_lock;
> > atomic_t ongoing_txs;
> > atomic_t hold_txs;
> > + atomic_t queue_stopped;
>
> Maybe some test_bit()/set_bit() is better there?
What do you mean? Shall I change the atomic_t type of queue_stopped?
Isn't the atomic_t preferred in this situation?
^ permalink raw reply
* Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
From: Oliver Hartkopp @ 2022-05-17 13:35 UTC (permalink / raw)
To: Max Staudt, Marc Kleine-Budde
Cc: Vincent MAILHOL, linux-can, linux-kernel, netdev
In-Reply-To: <20220517143921.08458f2c.max@enpas.org>
On 5/17/22 14:39, Max Staudt wrote:
> On Tue, 17 May 2022 14:21:53 +0200
> Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
>> On 17.05.2022 14:14:04, Max Staudt wrote:
>>>> After looking through drivers/net/can/Kconfig I would probably
>>>> phrase it like this:
>>>>
>>>> Select CAN devices (hw/sw) -> we compile a can_dev module. E.g.
>>>> to handle the skb stuff for vcan's.
>>>>
>>>> Select hardware CAN devices -> we compile the netlink stuff into
>>>> can_dev and offer CAN_CALC_BITTIMING and CAN_LEDS to be compiled
>>>> into can_dev too.
>>>>
>>>> In the latter case: The selection of flexcan, ti_hecc and
>>>> mcp251xfd automatically selects CAN_RX_OFFLOAD which is then also
>>>> compiled into can_dev.
>>>>
>>>> Would that fit in terms of complexity?
>>>
>>> IMHO these should always be compiled into can-dev. Out of tree
>>> drivers are fairly common here, and having to determine which kind
>>> of can-dev (stripped or not) the user has on their system is a
>>> nightmare waiting to happen.
>>
>> I personally don't care about out-of-tree drivers.
>
> I know that this is the official stance in the kernel.
>
> But out-of-tree drivers do happen on a regular basis, even when
> developing with the aim of upstreaming. And if a developer builds a
> minimal kernel to host a CAN driver, without building in-tree hardware
> CAN drivers, then can-dev will be there but behave differently from
> can-dev in a full distro. Leading to heisenbugs and wasting time. The
> source of heisenbugs really are the suggested *hidden* Kconfigs.
>
>
> On another note, is the module accounting overhead in the kernel for
> two new modules with relatively little code in each, code that almost
> always is loaded when CAN is used, really worth it?
Oh, I didn't want to introduce two new kernel modules but to have
can_dev in different 'feature levels'.
I would assume a distro kernel to have everything enabled with a full
featured can_dev - which is likely the base for out-of-tree drivers too.
But e.g. the people that are running Linux instances in a cloud only
using vcan and vxcan would not need to carry the entire infrastructure
of CAN hardware support and rx-offload.
Best regards,
Oliver
^ permalink raw reply
* Re: [PATCH wpan-next v2 09/11] net: mac802154: Introduce a synchronous API for MLME commands
From: Miquel Raynal @ 2022-05-17 13:30 UTC (permalink / raw)
To: Alexander Aring
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, Network Development, David Girault,
Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <CAK-6q+i_T+FaK0tX6tF38VjyEfSzDi-QC85MTU2=4soepAag8g@mail.gmail.com>
aahringo@redhat.com wrote on Sun, 15 May 2022 19:03:53 -0400:
> Hi,
>
> On Sun, May 15, 2022 at 6:28 PM Alexander Aring <aahringo@redhat.com> wrote:
> >
> > Hi,
> >
> > On Thu, May 12, 2022 at 10:34 AM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > >
> > > This is the slow path, we need to wait for each command to be processed
> > > before continuing so let's introduce an helper which does the
> > > transmission and blocks until it gets notified of its asynchronous
> > > completion. This helper is going to be used when introducing scan
> > > support.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > > net/mac802154/ieee802154_i.h | 1 +
> > > net/mac802154/tx.c | 25 +++++++++++++++++++++++++
> > > 2 files changed, 26 insertions(+)
> > >
> > > diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
> > > index a057827fc48a..f8b374810a11 100644
> > > --- a/net/mac802154/ieee802154_i.h
> > > +++ b/net/mac802154/ieee802154_i.h
> > > @@ -125,6 +125,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan;
> > > void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb);
> > > void ieee802154_xmit_sync_worker(struct work_struct *work);
> > > int ieee802154_sync_and_hold_queue(struct ieee802154_local *local);
> > > +int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb);
> > > netdev_tx_t
> > > ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev);
> > > netdev_tx_t
> > > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
> > > index 38f74b8b6740..ec8d872143ee 100644
> > > --- a/net/mac802154/tx.c
> > > +++ b/net/mac802154/tx.c
> > > @@ -128,6 +128,31 @@ int ieee802154_sync_and_hold_queue(struct ieee802154_local *local)
> > > return ieee802154_sync_queue(local);
> > > }
> > >
> > > +int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb)
> > > +{
> > > + int ret;
> > > +
> > > + /* Avoid possible calls to ->ndo_stop() when we asynchronously perform
> > > + * MLME transmissions.
> > > + */
> > > + rtnl_lock();
> >
> > I think we should make an ASSERT_RTNL() here, the lock needs to be
> > earlier than that over the whole MLME op. MLME can trigger more than
>
> not over the whole MLME_op, that's terrible to hold the rtnl lock so
> long... so I think this is fine that some netdev call will interfere
> with this transmission.
> So forget about the ASSERT_RTNL() here, it's fine (I hope).
>
> > one message, the whole sync_hold/release queue should be earlier than
> > that... in my opinion is it not right to allow other messages so far
> > an MLME op is going on? I am not sure what the standard says to this,
> > but I think it should be stopped the whole time? All those sequence
>
> Whereas the stop of the netdev queue makes sense for the whole mlme-op
> (in my opinion).
I might still implement an MLME pre/post helper and do the queue
hold/release calls there, while only taking the rtnl from the _tx.
And I might create an mlme_tx_one() which does the pre/post calls as
well.
Would something like this fit?
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH wpan-next v2 05/11] net: mac802154: Bring the hability to hold the transmit queue
From: Miquel Raynal @ 2022-05-17 13:28 UTC (permalink / raw)
To: Alexander Aring
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, open list:NETWORKING [GENERAL],
David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <CAK-6q+ihfOSBjpw1Q-2qesd4nkrAfw_rBCd0QcWzXk0PP9Prtg@mail.gmail.com>
aahringo@redhat.com wrote on Tue, 17 May 2022 09:19:29 -0400:
> Hi,
>
> On Tue, May 17, 2022 at 5:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alex,
> >
> > > > @@ -84,7 +118,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > > hw->phy->sifs_period * NSEC_PER_USEC,
> > > > HRTIMER_MODE_REL);
> > > > } else {
> > > > - ieee802154_wake_queue(hw);
> > > > + ieee802154_release_queue(local);
> > > > }
> > > >
> > > > dev_consume_skb_any(skb);
> > > > @@ -98,7 +132,7 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > > struct ieee802154_local *local = hw_to_local(hw);
> > > >
> > > > local->tx_result = reason;
> > > > - ieee802154_wake_queue(hw);
> > > > + ieee802154_release_queue(local);
> > > > dev_kfree_skb_any(skb);
> > > > atomic_dec(&hw->phy->ongoing_txs);
> > >
> > > I am pretty sure that will end in a scheduling while atomic warning
> > > with hwsim. If you don't hit it you have the wrong config, you need to
> > > enable such warnings and have the right preemption model setting.
> >
> > I was using the "desktop" kernel preemption model (voluntary), I've
> > switched to CONFIG_PREEMPT ("Preemptible kernel (Low-latency)"),
> > and enabled CONFIG_DEBUG_ATOMIC_SLEEP. You are right that we should use
> > a spinlock instead of a mutex here. However I don't think disabling
> > IRQs is necessary, so I'll switch to spin_(un)lock() calls.
> >
>
> In my opinion it's necessary for the ifs hrtimer. Normal
> spin_lock/unlock is not the right fit here.
You're right, I forgot about hrtimers.
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH 19/30] panic: Add the panic hypervisor notifier list
From: Petr Mladek @ 2022-05-17 13:28 UTC (permalink / raw)
To: Evan Green
Cc: Guilherme G. Piccoli, David Gow, Julius Werner, Scott Branden,
bcm-kernel-feedback-list, Sebastian Reichel, Linux PM,
Florian Fainelli, Andrew Morton, bhe, kexec, LKML, linuxppc-dev,
linux-alpha, linux-arm Mailing List, linux-edac, linux-hyperv,
linux-leds, linux-mips, linux-parisc, linux-remoteproc,
linux-s390, linux-tegra, linux-um, linux-xtensa, netdev,
openipmi-developer, rcu, sparclinux, xen-devel, x86, kernel-dev,
kernel, halves, fabiomirmar, alejandro.j.jimenez, Andy Shevchenko,
Arnd Bergmann, Borislav Petkov, Jonathan Corbet, d.hatayama,
dave.hansen, dyoung, feng.tang, Greg Kroah-Hartman, mikelley,
hidehiro.kawai.ez, jgross, john.ogness, Kees Cook, luto, mhiramat,
mingo, paulmck, peterz, rostedt, senozhatsky, Alan Stern,
Thomas Gleixner, vgoyal, vkuznets, Will Deacon, Alexander Gordeev,
Andrea Parri, Ard Biesheuvel, Benjamin Herrenschmidt,
Brian Norris, Christian Borntraeger, Christophe JAILLET,
David S. Miller, Dexuan Cui, Doug Berger, Haiyang Zhang,
Hari Bathini, Heiko Carstens, Justin Chen, K. Y. Srinivasan,
Lee Jones, Markus Mayer, Michael Ellerman, Mihai Carabas,
Nicholas Piggin, Paul Mackerras, Pavel Machek, Shile Zhang,
Stephen Hemminger, Sven Schnelle, Thomas Bogendoerfer, Tianyu Lan,
Vasily Gorbik, Wang ShaoBo, Wei Liu, zhenwei pi, Stephen Boyd
In-Reply-To: <CAE=gft7ds+dHfEkRz8rnSH1EbTpGTpKbi5Wxj9DW0Jr5mX_j4w@mail.gmail.com>
On Mon 2022-05-16 09:02:10, Evan Green wrote:
> On Mon, May 16, 2022 at 8:07 AM Guilherme G. Piccoli
> <gpiccoli@igalia.com> wrote:
> >
> > Thanks for the review!
> >
> > I agree with the blinking stuff, I can rework and add all LED/blinking
> > stuff into the loop list, it does make sense. I'll comment a bit in the
> > others below...
> >
> > On 16/05/2022 11:01, Petr Mladek wrote:
> > > [...]
> > >> --- a/arch/mips/sgi-ip22/ip22-reset.c
> > >> +++ b/arch/mips/sgi-ip22/ip22-reset.c
> > >> @@ -195,7 +195,7 @@ static int __init reboot_setup(void)
> > >> }
> > >>
> > >> timer_setup(&blink_timer, blink_timeout, 0);
> > >> - atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
> > >> + atomic_notifier_chain_register(&panic_hypervisor_list, &panic_block);
> > >
> > > This notifier enables blinking. It is not much safe. It calls
> > > mod_timer() that takes a lock internally.
> > >
> > > This kind of functionality should go into the last list called
> > > before panic() enters the infinite loop. IMHO, all the blinking
> > > stuff should go there.
> > > [...]
> > >> --- a/arch/mips/sgi-ip32/ip32-reset.c
> > >> +++ b/arch/mips/sgi-ip32/ip32-reset.c
> > >> @@ -145,7 +144,7 @@ static __init int ip32_reboot_setup(void)
> > >> pm_power_off = ip32_machine_halt;
> > >>
> > >> timer_setup(&blink_timer, blink_timeout, 0);
> > >> - atomic_notifier_chain_register(&panic_notifier_list, &panic_block);
> > >> + atomic_notifier_chain_register(&panic_hypervisor_list, &panic_block);
> > >
> > > Same here. Should be done only before the "loop".
> > > [...]
> >
> > Ack.
> >
> >
> > >> --- a/drivers/firmware/google/gsmi.c
> > >> +++ b/drivers/firmware/google/gsmi.c
> > >> @@ -1034,7 +1034,7 @@ static __init int gsmi_init(void)
> > >>
> > >> register_reboot_notifier(&gsmi_reboot_notifier);
> > >> register_die_notifier(&gsmi_die_notifier);
> > >> - atomic_notifier_chain_register(&panic_notifier_list,
> > >> + atomic_notifier_chain_register(&panic_hypervisor_list,
> > >> &gsmi_panic_notifier);
> > >
> > > I am not sure about this one. It looks like some logging or
> > > pre_reboot stuff.
> > >
> >
> > Disagree here. I'm looping Google maintainers, so they can comment.
> > (CCed Evan, David, Julius)
> >
> > This notifier is clearly a hypervisor notification mechanism. I've fixed
> > a locking stuff there (in previous patch), I feel it's low-risk but even
> > if it's mid-risk, the class of such callback remains a perfect fit with
> > the hypervisor list IMHO.
>
> This logs a panic to our "eventlog", a tiny logging area in SPI flash
> for critical and power-related events. In some cases this ends up
> being the only clue we get in a Chromebook feedback report that a
> panic occurred, so from my perspective moving it to the front of the
> line seems like a good idea.
IMHO, this would really better fit into the pre-reboot notifier list:
+ the callback stores the log so it is similar to kmsg_dump()
or console_flush_on_panic()
+ the callback should be proceed after "info" notifiers
that might add some other useful information.
Honestly, I am not sure what exactly hypervisor callbacks do. But I
think that they do not try to extract the kernel log because they
would need to handle the internal format.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH wpan-next v2 08/11] net: mac802154: Introduce a tx queue flushing mechanism
From: Miquel Raynal @ 2022-05-17 13:20 UTC (permalink / raw)
To: Alexander Aring
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, Network Development, David Girault,
Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <CAK-6q+iazXHZmf2vteXGEEpSXLLp9279g5JD2whBn-_FPL0piw@mail.gmail.com>
Hi Alex,
aahringo@redhat.com wrote on Sun, 15 May 2022 18:23:04 -0400:
> Hi,
>
> On Thu, May 12, 2022 at 10:34 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > Right now we are able to stop a queue but we have no indication if a
> > transmission is ongoing or not.
> >
> > Thanks to recent additions, we can track the number of ongoing
> > transmissions so we know if the last transmission is over. Adding on top
> > of it an internal wait queue also allows to be woken up asynchronously
> > when this happens. If, beforehands, we marked the queue to be held and
> > stopped it, we end up flushing and stopping the tx queue.
> >
> > Thanks to this feature, we will soon be able to introduce a synchronous
> > transmit API.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > include/net/cfg802154.h | 1 +
> > net/ieee802154/core.c | 1 +
> > net/mac802154/cfg.c | 2 +-
> > net/mac802154/ieee802154_i.h | 1 +
> > net/mac802154/tx.c | 26 ++++++++++++++++++++++++--
> > net/mac802154/util.c | 6 ++++--
> > 6 files changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > index ad3f438e4583..8b6326aa2d42 100644
> > --- a/include/net/cfg802154.h
> > +++ b/include/net/cfg802154.h
> > @@ -218,6 +218,7 @@ struct wpan_phy {
> > struct mutex queue_lock;
> > atomic_t ongoing_txs;
> > atomic_t hold_txs;
> > + wait_queue_head_t sync_txq;
> >
> > char priv[] __aligned(NETDEV_ALIGN);
> > };
> > diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
> > index d81b7301e013..f13e3082d988 100644
> > --- a/net/ieee802154/core.c
> > +++ b/net/ieee802154/core.c
> > @@ -129,6 +129,7 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size)
> > wpan_phy_net_set(&rdev->wpan_phy, &init_net);
> >
> > init_waitqueue_head(&rdev->dev_wait);
> > + init_waitqueue_head(&rdev->wpan_phy.sync_txq);
> >
> > mutex_init(&rdev->wpan_phy.queue_lock);
> >
> > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
> > index b51100fd9e3f..93df24f75572 100644
> > --- a/net/mac802154/cfg.c
> > +++ b/net/mac802154/cfg.c
> > @@ -46,7 +46,7 @@ static int ieee802154_suspend(struct wpan_phy *wpan_phy)
> > if (!local->open_count)
> > goto suspend;
> >
> > - ieee802154_hold_queue(local);
> > + ieee802154_sync_and_hold_queue(local);
> > synchronize_net();
> >
> > /* stop hardware - this must stop RX */
> > diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
> > index e34db1d49ef4..a057827fc48a 100644
> > --- a/net/mac802154/ieee802154_i.h
> > +++ b/net/mac802154/ieee802154_i.h
> > @@ -124,6 +124,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan;
> >
> > void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb);
> > void ieee802154_xmit_sync_worker(struct work_struct *work);
> > +int ieee802154_sync_and_hold_queue(struct ieee802154_local *local);
> > netdev_tx_t
> > ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev);
> > netdev_tx_t
> > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
> > index 607019b8f8ab..38f74b8b6740 100644
> > --- a/net/mac802154/tx.c
> > +++ b/net/mac802154/tx.c
> > @@ -44,7 +44,8 @@ void ieee802154_xmit_sync_worker(struct work_struct *work)
> > err_tx:
> > /* Restart the netif queue on each sub_if_data object. */
> > ieee802154_release_queue(local);
> > - atomic_dec(&local->phy->ongoing_txs);
> > + if (!atomic_dec_and_test(&local->phy->ongoing_txs))
> > + wake_up(&local->phy->sync_txq);
> > kfree_skb(skb);
> > netdev_dbg(dev, "transmission failed\n");
> > }
> > @@ -100,12 +101,33 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb)
> >
> > err_wake_netif_queue:
> > ieee802154_release_queue(local);
> > - atomic_dec(&local->phy->ongoing_txs);
> > + if (!atomic_dec_and_test(&local->phy->ongoing_txs))
> > + wake_up(&local->phy->sync_txq);
> > err_free_skb:
> > kfree_skb(skb);
> > return NETDEV_TX_OK;
> > }
> >
> > +static int ieee802154_sync_queue(struct ieee802154_local *local)
> > +{
> > + int ret;
> > +
> > + ieee802154_hold_queue(local);
> > + ieee802154_disable_queue(local);
> > + wait_event(local->phy->sync_txq, !atomic_read(&local->phy->ongoing_txs));
> > + ret = local->tx_result;
> > + ieee802154_release_queue(local);
>
> I am curious, why this extra hold, release here?
My idea was:
- stop the queue
- increment the hold counter to be sure the queue does not get
restarted asynchronously
- wait for the last transmission to finish
- decrement the hold counter
- restart the queue if the hold counter is null
What is bothering you with it? Without the hold we cannot be sure that
an asynchronous event will not restart the queue and possibly fail our
logic.
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH v2] net: phy: marvell: Add errata section 5.1 for Alaska PHY
From: patchwork-bot+netdevbpf @ 2022-05-17 13:20 UTC (permalink / raw)
To: Stefan Roese; +Cc: netdev, lpolak, kabel, andrew, hkallweit1, linux, davem
In-Reply-To: <20220516070859.549170-1-sr@denx.de>
Hello:
This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 16 May 2022 09:08:59 +0200 you wrote:
> From: Leszek Polak <lpolak@arri.de>
>
> As per Errata Section 5.1, if EEE is intended to be used, some register
> writes must be done once after every hardware reset. This patch now adds
> the necessary register writes as listed in the Marvell errata.
>
> Without this fix we experience ethernet problems on some of our boards
> equipped with a new version of this ethernet PHY (different supplier).
>
> [...]
Here is the summary with links:
- [v2] net: phy: marvell: Add errata section 5.1 for Alaska PHY
https://git.kernel.org/netdev/net-next/c/65a9dedc11d6
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 wpan-next v2 05/11] net: mac802154: Bring the hability to hold the transmit queue
From: Alexander Aring @ 2022-05-17 13:19 UTC (permalink / raw)
To: Miquel Raynal
Cc: Alexander Aring, Stefan Schmidt, linux-wpan - ML, David S. Miller,
Jakub Kicinski, Paolo Abeni, open list:NETWORKING [GENERAL],
David Girault, Romuald Despres, Frederic Blain, Nicolas Schodet,
Thomas Petazzoni
In-Reply-To: <20220517112726.4b89e907@xps-13>
Hi,
On Tue, May 17, 2022 at 5:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> > > @@ -84,7 +118,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > hw->phy->sifs_period * NSEC_PER_USEC,
> > > HRTIMER_MODE_REL);
> > > } else {
> > > - ieee802154_wake_queue(hw);
> > > + ieee802154_release_queue(local);
> > > }
> > >
> > > dev_consume_skb_any(skb);
> > > @@ -98,7 +132,7 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > struct ieee802154_local *local = hw_to_local(hw);
> > >
> > > local->tx_result = reason;
> > > - ieee802154_wake_queue(hw);
> > > + ieee802154_release_queue(local);
> > > dev_kfree_skb_any(skb);
> > > atomic_dec(&hw->phy->ongoing_txs);
> >
> > I am pretty sure that will end in a scheduling while atomic warning
> > with hwsim. If you don't hit it you have the wrong config, you need to
> > enable such warnings and have the right preemption model setting.
>
> I was using the "desktop" kernel preemption model (voluntary), I've
> switched to CONFIG_PREEMPT ("Preemptible kernel (Low-latency)"),
> and enabled CONFIG_DEBUG_ATOMIC_SLEEP. You are right that we should use
> a spinlock instead of a mutex here. However I don't think disabling
> IRQs is necessary, so I'll switch to spin_(un)lock() calls.
>
In my opinion it's necessary for the ifs hrtimer. Normal
spin_lock/unlock is not the right fit here.
- Alex
^ 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