* Re: [PATCH net-next] net: sfp: add debugfs support
From: Russell King - ARM Linux admin @ 2020-11-24 9:49 UTC (permalink / raw)
To: Ido Schimmel
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, netdev,
Jakub Kicinski
In-Reply-To: <20201124084151.GA722671@shredder.lan>
On Tue, Nov 24, 2020 at 10:41:51AM +0200, Ido Schimmel wrote:
> On Tue, Nov 24, 2020 at 01:14:31AM +0100, Andrew Lunn wrote:
> > On Mon, Nov 23, 2020 at 10:06:16PM +0000, Russell King wrote:
> > > Add debugfs support to SFP so that the internal state of the SFP state
> > > machines and hardware signal state can be viewed from userspace, rather
> > > than having to compile a debug kernel to view state state transitions
> > > in the kernel log. The 'state' output looks like:
> > >
> > > Module state: empty
> > > Module probe attempts: 0 0
> > > Device state: up
> > > Main state: down
> > > Fault recovery remaining retries: 5
> > > PHY probe remaining retries: 12
> > > moddef0: 0
> > > rx_los: 1
> > > tx_fault: 1
> > > tx_disable: 1
> > >
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> >
> > Hi Russell
> >
> > This looks useful. I always seem to end up recompiling the kernel,
> > which as you said, this should avoid.
>
> FWIW, another option is to use drgn [1]. Especially when the state is
> queried from the kernel and not hardware. We are using that in mlxsw
> [2][3].
Presumably that requires /proc/kcore support, which 32-bit ARM doesn't
have.
--
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 v15 6/9] arm64/kvm: Add hypercall service for kvm ptp.
From: Jianyong Wu @ 2020-11-24 10:14 UTC (permalink / raw)
To: Marc Zyngier
Cc: netdev@vger.kernel.org, yangbo.lu@nxp.com, john.stultz@linaro.org,
tglx@linutronix.de, pbonzini@redhat.com,
sean.j.christopherson@intel.com, richardcochran@gmail.com,
Mark Rutland, will@kernel.org, Suzuki Poulose, Andre Przywara,
Steven Price, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Steve Capper,
Justin He, nd
In-Reply-To: <9133f26699c5fc08d0ea72acfa9aca3b@kernel.org>
Hi Marc,
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Tuesday, November 24, 2020 5:07 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; pbonzini@redhat.com; sean.j.christopherson@intel.com;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>;
> will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Andre
> Przywara <Andre.Przywara@arm.com>; Steven Price
> <Steven.Price@arm.com>; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; Steve Capper <Steve.Capper@arm.com>; Justin He
> <Justin.He@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v15 6/9] arm64/kvm: Add hypercall service for kvm ptp.
>
> On 2020-11-24 05:20, Jianyong Wu wrote:
> > Hi Marc,
>
> [...]
>
> >> > +/* ptp_kvm counter type ID */
> >> > +#define ARM_PTP_VIRT_COUNTER 0
> >> > +#define ARM_PTP_PHY_COUNTER 1
> >> > +#define ARM_PTP_NONE_COUNTER 2
> >>
> >> The architecture definitely doesn't have this last counter.
> >
> > Yeah, this is just represent no counter data needed from guest.
> > Some annotation should be added here.
>
> I'd rather you remove it entirely, or explain why you really cannot do without
> a fake counter.
OK, I will remove it.
Thanks
Jianyong
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply
* RE: [PATCH v15 7/9] ptp: arm/arm64: Enable ptp_kvm for arm/arm64
From: Jianyong Wu @ 2020-11-24 10:18 UTC (permalink / raw)
To: Marc Zyngier
Cc: netdev@vger.kernel.org, yangbo.lu@nxp.com, john.stultz@linaro.org,
tglx@linutronix.de, pbonzini@redhat.com,
sean.j.christopherson@intel.com, richardcochran@gmail.com,
Mark Rutland, will@kernel.org, Suzuki Poulose, Andre Przywara,
Steven Price, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Steve Capper,
Justin He, nd
In-Reply-To: <5dc5480d125ace6566ae616206c3ce3f@kernel.org>
Hi Marc,
> -----Original Message-----
> From: Marc Zyngier <maz@kernel.org>
> Sent: Tuesday, November 24, 2020 5:05 PM
> To: Jianyong Wu <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; pbonzini@redhat.com; sean.j.christopherson@intel.com;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>;
> will@kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Andre
> Przywara <Andre.Przywara@arm.com>; Steven Price
> <Steven.Price@arm.com>; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> kvm@vger.kernel.org; Steve Capper <Steve.Capper@arm.com>; Justin He
> <Justin.He@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v15 7/9] ptp: arm/arm64: Enable ptp_kvm for
> arm/arm64
>
> Jianyong,
>
> On 2020-11-24 05:37, Jianyong Wu wrote:
> > Hi Marc,
>
> [...]
>
> >> > +
> >> arm_smccc_1_1_invoke(ARM_SMCCC_VENDOR_HYP_KVM_PTP_FU
> >> NC_ID,
> >> > + ARM_PTP_NONE_COUNTER, &hvc_res);
> >>
> >> I really don't see the need to use a non-architectural counter ID.
> >> Using the virtual counter ID should just be fine, and shouldn't lead
> >> to any issue.
> >>
> >
> >> Am I missing something?
> >
> > In this function, no counter data is needed. If virtual counter ID is
> > used here, user may be confused with why we get virtual counter
> > data and do nothing with it. So I explicitly add a new "NONE" counter
> > ID to make it clear.
> >
> > WDYT?
>
> ITIABI (I Think It's A Bad Idea). There are two counters, and the API
> allows to retrieve the data for any of these two. If the "user" doesn't
> want to do anything with the data, that's their problem.
>
> Here, you can just sue the virtual counter, and that will give you the
> exact same semantic, without inventing non-architectural state.
>
OK, that's it.
Thanks
Jianyong Wu
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] lockdep: Introduce in_softirq lockdep assert
From: Yunsheng Lin @ 2020-11-24 10:30 UTC (permalink / raw)
To: Peter Zijlstra, Jakub Kicinski
Cc: mingo, will, viro, kyk.segfault, davem, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed, netdev, linux-kernel, linuxarm,
Thomas Gleixner
In-Reply-To: <20201124081112.GF2414@hirez.programming.kicks-ass.net>
On 2020/11/24 16:11, Peter Zijlstra wrote:
> On Mon, Nov 23, 2020 at 12:12:59PM -0800, Jakub Kicinski wrote:
>> One liner would be:
>>
>> * Acceptable for protecting per-CPU resources accessed from BH
>>
>> We can add:
>>
>> * Much like in_softirq() - semantics are ambiguous, use carefully. *
>>
>>
>> IIUC we basically want to protect the nc array and counter here:
>
> Works for me, thanks!
Will add the above comment in v3.
Thanks.
> .
>
^ permalink raw reply
* Re: [PATCH 0/3] xsk: fix for xsk_poll writeable
From: Magnus Karlsson @ 2020-11-24 10:38 UTC (permalink / raw)
To: Xuan Zhuo
Cc: Björn Töpel, Magnus Karlsson, Jonathan Lemon,
David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Network Development, bpf, linux-kernel
In-Reply-To: <CAJ8uoz1yxjYyfrKkvJrjLWOzm78M2CtNVRC+GkeGRCWBq5xAYA@mail.gmail.com>
On Tue, Nov 24, 2020 at 10:01 AM Magnus Karlsson
<magnus.karlsson@gmail.com> wrote:
>
> On Mon, Nov 23, 2020 at 4:21 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Mon, 23 Nov 2020 15:00:48 +0100, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > > On Wed, Nov 18, 2020 at 9:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >
> > > > I tried to combine cq available and tx writeable, but I found it very difficult.
> > > > Sometimes we pay attention to the status of "available" for both, but sometimes,
> > > > we may only pay attention to one, such as tx writeable, because we can use the
> > > > item of fq to write to tx. And this kind of demand may be constantly changing,
> > > > and it may be necessary to set it every time before entering xsk_poll, so
> > > > setsockopt is not very convenient. I feel even more that using a new event may
> > > > be a better solution, such as EPOLLPRI, I think it can be used here, after all,
> > > > xsk should not have OOB data ^_^.
> > > >
> > > > However, two other problems were discovered during the test:
> > > >
> > > > * The mask returned by datagram_poll always contains EPOLLOUT
> > > > * It is not particularly reasonable to return EPOLLOUT based on tx not full
> > > >
> > > > After fixing these two problems, I found that when the process is awakened by
> > > > EPOLLOUT, the process can always get the item from cq.
> > > >
> > > > Because the number of packets that the network card can send at a time is
> > > > actually limited, suppose this value is "nic_num". Once the number of
> > > > consumed items in the tx queue is greater than nic_num, this means that there
> > > > must also be new recycled items in the cq queue from nic.
> > > >
> > > > In this way, as long as the tx configured by the user is larger, we won't have
> > > > the situation that tx is already in the writeable state but cannot get the item
> > > > from cq.
> > >
> > > I think the overall approach of tying this into poll() instead of
> > > setsockopt() is the right way to go. But we need a more robust
> > > solution. Your patch #3 also breaks backwards compatibility and that
> > > is not allowed. Could you please post some simple code example of what
> > > it is you would like to do in user space? So you would like to wake up
> > > when there are entries in the cq that can be retrieved and the reason
> > > you would like to do this is that you then know you can put some more
> > > entries into the Tx ring and they will get sent as there now are free
> > > slots in the cq. Correct me if wrong. Would an event that wakes you up
> > > when there is both space in the Tx ring and space in the cq work? Is
> > > there a case in which we would like to be woken up when only the Tx
> > > ring is non-full? Maybe there are as it might be beneficial to fill
> > > the Tx and while doing that some entries in the cq has been completed
> > > and away the packets go. But it would be great if you could post some
> > > simple example code, does not need to compile or anything. Can be
> > > pseudo code.
> > >
> > > It would also be good to know if your goal is max throughput, max
> > > burst size, or something else.
> > >
> > > Thanks: Magnus
> > >
> >
> > My goal is max pps, If possible, increase the size of buf appropriately to
> > improve throughput. like pktgen.
> >
> > The code like this: (tx and umem cq also is 1024, and that works with zero
> > copy.)
> >
> > ```
> > void send_handler(xsk)
> > {
> > char buf[22];
> >
> > while (true) {
> > while (true){
> > if (send_buf_to_tx_ring(xsk, buf, sizeof(buf)))
> > break; // break this when no cq or tx is full
> > }
> >
> > if (sendto(xsk->fd))
> > break;
> > }
> > }
> > }
> >
> >
> > static int loop(int efd, xsk)
> > {
> > struct epoll_event e[1024];
> > struct epoll_event ee;
> > int n, i;
> >
> > ee.events = EPOLLOUT;
> > ee.data.ptr = NULL;
> >
> > epoll_ctl(efd, EPOLL_CTL_ADD, xsk->fd, &e);
> >
> > while (1) {
> > n = epoll_wait(efd, e, sizeof(e)/sizeof(e[0]), -1);
> >
> > if (n == 0)
> > continue;
> >
> > if (n < 0) {
> > continue;
> > }
> >
> > for (i = 0; i < n; ++i) {
> > send_handler(xsk);
> > }
> > }
> > }
> > ```
> >
> > 1. Now, since datagram_poll(that determine whether it is write able based on
> > sock_writeable function) will return EPOLLOUT every time, epoll_wait will
> > always return directly(this results in cpu 100%).
>
> We should keep patch #1. Just need to make sure we do not break
> anything as I am not familiar with the path after xsk_poll returns.
>
> > 2. After removing datagram_poll, since tx full is a very short moment, so every
> > time tx is not full is always true, epoll_wait will still return directly
> > 3. After epoll_wait returns, app will try to get cq and writes it to tx again,
> > but this time basically it will fail when getting cq. My analysis is that
> > cq item has not returned from the network card at this time.
> >
> >
> > Under normal circumstances, the judgment preparation for this event that can be
> > written is not whether the queue or buffer is full. The judgment criterion of
> > tcp is whether the free space is more than half.
> > This is the origin of my #2 patch, and I found that after adding this patch, my
> > above problems no longer appear.
> > 1. epoll_wait no longer exits directly
> > 2. Every time you receive EPOLLOUT, you can always get cq
>
> Got it. Make sense. And good that there is some precedence that you
> are not supposed to wake up when there is one free slot. Instead you
> should wake up when a lot of them are free so you can insert a batch.
> So let us also keep patch #2, though I might have some comments on it,
> but I will reply to that patch in that case.
>
> But patch #3 needs to go. How about you instead make the Tx ring
> double the size of the completion ring? Let us assume patch #1 and #2
> are in place. You will get woken up when at least half the entries in
> the Tx ring are available. At this point fill the Tx ring completely
> and after that start cleaning the completion ring. Hopefully by this
> time, there will be a number of entries in there that can be cleaned
> up. Then you call sendto(). It might even be a good idea to do cq, Tx,
> cq in that order.
>
> I consider #1 and #2 bug fixes so please base them on the bpf tree and
> note this in your mail header like this: "[PATCH bpf 0/3] xsk: fix for
> xsk_poll writeable".
>
> >
> > In addition:
> > What is the goal of TX_BATCH_SIZE and why this "restriction" should be added,
> > which causes a lot of trouble in programming without using zero copy
>
> You are right, this is likely too low. I never thought of this as
> something that would be used as a "fast-path". It was only a generic
> fall back. But it need not be. Please produce a patch #3 that sets
> this to a higher value. We do need the limit though. How about 512?
Please produce one patch set with #1 and #2 based on the bpf tree and
keep the TX_BATCH_SIZE patch separate. It is only a performance
optimization and should be based on bpf-next and sent as a sole patch
on its own.
Thanks!
> If you are interested in improving the performance of the Tx SKB path,
> then there might be other avenues to try if you are interested. Here
> are some examples:
>
> * Batch dev_direct_xmit. Maybe skb lists can be used.
> * Do not unlock and lock for every single packet in dev_direct_xmit().
> Can be combined with the above.
> * Use fragments instead of copying packets into the skb itself
> * Can the bool more in netdev_start_xmit be used to increase performance
>
> >
> > Thanks.
> >
> > >
> > > > Xuan Zhuo (3):
> > > > xsk: replace datagram_poll by sock_poll_wait
> > > > xsk: change the tx writeable condition
> > > > xsk: set tx/rx the min entries
> > > >
> > > > include/uapi/linux/if_xdp.h | 2 ++
> > > > net/xdp/xsk.c | 26 ++++++++++++++++++++++----
> > > > net/xdp/xsk_queue.h | 6 ++++++
> > > > 3 files changed, 30 insertions(+), 4 deletions(-)
> > > >
> > > > --
> > > > 1.8.3.1
> > > >
^ permalink raw reply
* [PATCH 1/1] tools/bpftool: fix error return value in build_btf_type_table()
From: Zhen Lei @ 2020-11-24 10:41 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, bpf, linux-kernel
Cc: Zhen Lei
An appropriate return value should be set on the failed path.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
tools/bpf/bpftool/btf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index 8ab142ff5eac..2afb7d5b1aca 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -693,6 +693,7 @@ build_btf_type_table(struct btf_attach_table *tab, enum bpf_obj_type type,
obj_node = calloc(1, sizeof(*obj_node));
if (!obj_node) {
p_err("failed to allocate memory: %s", strerror(errno));
+ err = -ENOMEM;
goto err_free;
}
--
2.26.0.106.g9fadedd
^ permalink raw reply related
* Re: [PATCH] usbnet: ipheth: fix connectivity with iOS 14
From: Yves-Alexis Perez @ 2020-11-24 10:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S. Miller, Martin Habets, Luc Van Oostenryck,
Shannon Nelson, Michael S. Tsirkin, linux-usb, netdev,
linux-kernel, Matti Vuorela, stable
In-Reply-To: <20201121140311.42585c68@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Sat, 2020-11-21 at 14:03 -0800, Jakub Kicinski wrote:
> Applied to net with the typo fixed, thanks!
Thanks!
Is there any chance it'll be in 5.10 or will it have to wait for the 5.11
merge window?
Also it should be applied to all supported/stable kernels. I guess that'll
have to wait until it's in Linus tree according [1] to but I'm unsure if I
need to trigger the action myself or if Greg (or Dave, according to [2]) will
do it.
I looked at [3] and it seems that adding the CC: stable in my commit message
maybe was an error because it's marked as a Failure, so if there's anything
needed from me here, don't hesitate to ask.
[1] https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
[2]
https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#q-how-can-i-tell-what-patches-are-queued-up-for-backporting-to-the-various-stable-releases
[3] https://patchwork.kernel.org/bundle/netdev/stable/?state=*
Regards,
--
Yves-Alexis
^ permalink raw reply
* Re: [PATCH net-next] net: sfp: add debugfs support
From: Ido Schimmel @ 2020-11-24 10:46 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, netdev,
Jakub Kicinski
In-Reply-To: <20201124094916.GD1551@shell.armlinux.org.uk>
On Tue, Nov 24, 2020 at 09:49:16AM +0000, Russell King - ARM Linux admin wrote:
> On Tue, Nov 24, 2020 at 10:41:51AM +0200, Ido Schimmel wrote:
> > On Tue, Nov 24, 2020 at 01:14:31AM +0100, Andrew Lunn wrote:
> > > On Mon, Nov 23, 2020 at 10:06:16PM +0000, Russell King wrote:
> > > > Add debugfs support to SFP so that the internal state of the SFP state
> > > > machines and hardware signal state can be viewed from userspace, rather
> > > > than having to compile a debug kernel to view state state transitions
> > > > in the kernel log. The 'state' output looks like:
> > > >
> > > > Module state: empty
> > > > Module probe attempts: 0 0
> > > > Device state: up
> > > > Main state: down
> > > > Fault recovery remaining retries: 5
> > > > PHY probe remaining retries: 12
> > > > moddef0: 0
> > > > rx_los: 1
> > > > tx_fault: 1
> > > > tx_disable: 1
> > > >
> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > >
> > > Hi Russell
> > >
> > > This looks useful. I always seem to end up recompiling the kernel,
> > > which as you said, this should avoid.
> >
> > FWIW, another option is to use drgn [1]. Especially when the state is
> > queried from the kernel and not hardware. We are using that in mlxsw
> > [2][3].
>
> Presumably that requires /proc/kcore support, which 32-bit ARM doesn't
> have.
Yes, it does seem to be required for live debugging. I mostly work with
x86 systems, I guess it's completely different for Andrew and you.
^ permalink raw reply
* Re: [PATCH net] netdevice.h: Fix unintentional disable of ALL_FOR_ALL features on upper device
From: Eric Dumazet @ 2020-11-24 10:48 UTC (permalink / raw)
To: Tariq Toukan, Herbert Xu
Cc: Tariq Toukan, David S. Miller, Jakub Kicinski, netdev,
Moshe Shemesh, Maxim Mikityanskiy, Saeed Mahameed
In-Reply-To: <9bf8ba40-cd40-2af6-d358-48dd98526434@gmail.com>
On Mon, Nov 23, 2020 at 5:15 PM Tariq Toukan <ttoukan.linux@gmail.com> wrote:
>
>
>
> On 11/23/2020 4:55 PM, Eric Dumazet wrote:
> > On Mon, Nov 23, 2020 at 3:13 PM Tariq Toukan <tariqt@nvidia.com> wrote:
> >>
> >> Calling netdev_increment_features() on upper/master device from
> >> netdev_add_tso_features() implies unintentional clearance of ALL_FOR_ALL
> >> features supported by all slaves. Fix it by passing ALL_FOR_ALL in
> >> addition to ALL_TSO.
> >>
> >> Fixes: b0ce3508b25e ("bonding: allow TSO being set on bonding master")
> >
> > I think you should give more details to your bug report, because
> > netdev_add_tso_features() is used from different
> > places.
> >
> > Thanks.
> >
>
> Right. I'll include these in the re-spin:
> Fixes: 247f6d0f8667 ("team: allow TSO being set on master")
> Fixes: f902e8812ef6 ("bridge: Add ability to enable TSO")
I was more thinking about what exact issue you had, and how we can
reproduce it, and test the fix.
>
> I wonder though if netdev_increment_features() is expected to clear
> features that are not part of the mask.
Well, the 'increment' part was suggesting the function was adding
flags, not removing them.
We might ask Herbert Xu if we :
1) Need to comment the function, or change its name to be more descriptive.
2) Change the behavior (as you suggested)
3) Other choice.
^ permalink raw reply
* Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink message format
From: Eelco Chaudron @ 2020-11-24 10:51 UTC (permalink / raw)
To: Pravin Shelar
Cc: Linux Kernel Network Developers, David S. Miller, ovs dev,
Jakub Kicinski, Bindiya Kurle, Ilya Maximets, mcroce
In-Reply-To: <CAOrHB_Be-B8oLwx-zYXpwhjpQAWdkw1NrYh36S8e6bRH8X0cqg@mail.gmail.com>
On 20 Nov 2020, at 23:16, Pravin Shelar wrote:
> On Thu, Nov 19, 2020 at 1:04 AM Eelco Chaudron <echaudro@redhat.com>
> wrote:
>>
>> Currently, the openvswitch module is not accepting the correctly
>> formated
>> netlink message for the TTL decrement action. For both setting and
>> getting
>> the dec_ttl action, the actions should be nested in the
>> OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h
>> uapi.
>>
>> Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> Thanks for working on this. can you share OVS kmod unit test for this
> action?
Hi Pravin,
I did add a self-test, however, my previous plan was to send out the
updated OVS patch after this change got accepted. But due to all the
comments, I sent it out anyway, so here it is with a datapath test:
https://mail.openvswitch.org/pipermail/ovs-dev/2020-November/377795.html
//Eelco
^ permalink raw reply
* [PATCH net-next v3 1/2] lockdep: Introduce in_softirq lockdep assert
From: Yunsheng Lin @ 2020-11-24 10:49 UTC (permalink / raw)
To: peterz, mingo, will, viro, kyk.segfault, davem, kuba, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed
Cc: netdev, linux-kernel, linuxarm
In-Reply-To: <1606214969-97849-1-git-send-email-linyunsheng@huawei.com>
The current semantic for napi_consume_skb() is that caller need
to provide non-zero budget when calling from NAPI context, and
breaking this semantic will cause hard to debug problem, because
_kfree_skb_defer() need to run in atomic context in order to push
the skb to the particular cpu' napi_alloc_cache atomically.
So add the lockdep_assert_in_softirq() to assert when the running
context is not in_softirq, in_softirq means softirq is serving or
BH is disabled, which has a ambiguous semantics due to the BH
disabled confusion, so add a comment to emphasize that.
And the softirq context can be interrupted by hard IRQ or NMI
context, lockdep_assert_in_softirq() need to assert about hard
IRQ or NMI context too.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
V3: add comment to emphasize the ambiguous semantics.
---
include/linux/lockdep.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index f559487..8d60f46 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -594,6 +594,13 @@ do { \
this_cpu_read(hardirqs_enabled))); \
} while (0)
+/* Much like in_softirq() - semantics are ambiguous, use carefully. */
+#define lockdep_assert_in_softirq() \
+do { \
+ WARN_ON_ONCE(__lockdep_enabled && \
+ (!in_softirq() || in_irq() || in_nmi())); \
+} while (0)
+
#else
# define might_lock(lock) do { } while (0)
# define might_lock_read(lock) do { } while (0)
@@ -605,6 +612,7 @@ do { \
# define lockdep_assert_preemption_enabled() do { } while (0)
# define lockdep_assert_preemption_disabled() do { } while (0)
+# define lockdep_assert_in_softirq() do { } while (0)
#endif
#ifdef CONFIG_PROVE_RAW_LOCK_NESTING
--
2.8.1
^ permalink raw reply related
* [PATCH net-next v3 2/2] net: Use lockdep_assert_in_softirq() in napi_consume_skb()
From: Yunsheng Lin @ 2020-11-24 10:49 UTC (permalink / raw)
To: peterz, mingo, will, viro, kyk.segfault, davem, kuba, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed
Cc: netdev, linux-kernel, linuxarm
In-Reply-To: <1606214969-97849-1-git-send-email-linyunsheng@huawei.com>
Use napi_consume_skb() to assert the case when it is not called
in a atomic softirq context.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
net/core/skbuff.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ffe3dcc..effa19d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -902,6 +902,8 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
return;
}
+ lockdep_assert_in_softirq();
+
if (!skb_unref(skb))
return;
--
2.8.1
^ permalink raw reply related
* [PATCH net-next v3 0/2] Add an assert in napi_consume_skb()
From: Yunsheng Lin @ 2020-11-24 10:49 UTC (permalink / raw)
To: peterz, mingo, will, viro, kyk.segfault, davem, kuba, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed
Cc: netdev, linux-kernel, linuxarm
This patch introduces a lockdep_assert_in_softirq() interface and
uses it to assert the case when napi_consume_skb() is not called in
the softirq context.
Changelog:
V3: add comment to emphasize the ambiguous semantics
V2: Use lockdep instead of one-off Kconfig knob
Yunsheng Lin (2):
lockdep: Introduce in_softirq lockdep assert
net: Use lockdep_assert_in_softirq() in napi_consume_skb()
include/linux/lockdep.h | 8 ++++++++
net/core/skbuff.c | 2 ++
2 files changed, 10 insertions(+)
--
2.8.1
^ permalink raw reply
* Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink message format
From: Eelco Chaudron @ 2020-11-24 11:18 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, dev, pshelar, bindiyakurle, i.maximets, mcroce
In-Reply-To: <20201120131228.489c3b52@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On 20 Nov 2020, at 22:12, Jakub Kicinski wrote:
> On Thu, 19 Nov 2020 04:04:04 -0500 Eelco Chaudron wrote:
>> Currently, the openvswitch module is not accepting the correctly
>> formated
>> netlink message for the TTL decrement action. For both setting and
>> getting
>> the dec_ttl action, the actions should be nested in the
>> OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h
>> uapi.
>
> IOW this change will not break any known user space, correct?
It will not as there isn’t any yet. Unfortunately, the original patch
was sent out without a userspace part. It was internally tested by the
original authors and not properly reviewed to bring forward the issue.
They did add some weird code to work around it.
> But existing OvS user space already expects it to work like you
> make it work now?
>
> What's the harm in leaving it as is?
Without this change, the different Datapaths in OVS behave differently,
making the code to be datapath agnostic having to do all kinds of weird
tricks to work around it.
But even worse, the patch in the current format could interpret
additional options/attributes as actions, due to the actions not being
encapsulated/nested within the actual attribute.
>> Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>
> Can we get a review from OvS folks? Matteo looks good to you (as the
> original author)?
See Matteo’s reply, looks like he is ok with this change.
>> - err = __ovs_nla_copy_actions(net, attr, key, sfa, eth_type,
>> + err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
>> vlan_tci, mpls_label_count, log);
>> if (err)
>> return err;
>
> You're not canceling any nests on error, I assume this is normal.
Yes, on error the sfa actions are not used.
>> + add_nested_action_end(*sfa, action_start);
>> add_nested_action_end(*sfa, start);
>> return 0;
>> }
^ permalink raw reply
* Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink message format
From: Eelco Chaudron @ 2020-11-24 11:19 UTC (permalink / raw)
To: Matteo Croce
Cc: Jakub Kicinski, netdev, David Miller, dev, Pravin B Shelar,
bindiyakurle, Ilya Maximets
In-Reply-To: <CAFnufp1RRtwDLwrWayvyZVPmDjab_dTx50u7xWeNwK7J6azqWw@mail.gmail.com>
On 23 Nov 2020, at 20:36, Matteo Croce wrote:
> On Fri, Nov 20, 2020 at 10:12 PM Jakub Kicinski <kuba@kernel.org>
> wrote:
>>
>> On Thu, 19 Nov 2020 04:04:04 -0500 Eelco Chaudron wrote:
>>> Currently, the openvswitch module is not accepting the correctly
>>> formated
>>> netlink message for the TTL decrement action. For both setting and
>>> getting
>>> the dec_ttl action, the actions should be nested in the
>>> OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h
>>> uapi.
>>
>> IOW this change will not break any known user space, correct?
>>
>> But existing OvS user space already expects it to work like you
>> make it work now?
>>
>> What's the harm in leaving it as is?
>>
>>> Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>
>> Can we get a review from OvS folks? Matteo looks good to you (as the
>> original author)?
>>
>
> Hi,
>
> I think that the userspace still has to implement the dec_ttl action;
> by now dec_ttl is implemented with set_ttl().
> So there is no breakage yet.
Yes, see reply to Jakub’s email.
> Eelco, with this fix we will encode the netlink attribute in the same
> way for the kernel and netdev datapath?
Yes, this should make both implementations the same. No more weird code
in the data-plane agnostic code :)
> If so, go for it.
>
>
>>> - err = __ovs_nla_copy_actions(net, attr, key, sfa, eth_type,
>>> + err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
>>> vlan_tci, mpls_label_count, log);
>>> if (err)
>>> return err;
>>
>> You're not canceling any nests on error, I assume this is normal.
>>
>>> + add_nested_action_end(*sfa, action_start);
>>> add_nested_action_end(*sfa, start);
>>> return 0;
>>> }
>>
>
>
> --
> per aspera ad upstream
^ permalink raw reply
* [PATCH net-next] mptcp: be careful on MPTCP-level ack.
From: Paolo Abeni @ 2020-11-24 11:20 UTC (permalink / raw)
To: netdev; +Cc: Jakub Kicinski, mptcp, Eric Dumazet
We can enter the main mptcp_recvmsg() loop even when
no subflows are connected. As note by Eric, that would
result in a divide by zero oops on ack generation.
Address the issue by checking the subflow status before
sending the ack.
Additionally protect mptcp_recvmsg() against invocation
with weird socket states.
Reported-and-suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Fixes: ea4ca586b16f ("mptcp: refine MPTCP-level ack scheduling")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/mptcp/protocol.c | 67 ++++++++++++++++++++++++++++++++------------
1 file changed, 49 insertions(+), 18 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 4b7794835fea..b50164efb741 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -419,31 +419,57 @@ static bool mptcp_subflow_active(struct mptcp_subflow_context *subflow)
return ((1 << ssk->sk_state) & (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT));
}
-static void mptcp_send_ack(struct mptcp_sock *msk, bool force)
+static inline bool tcp_can_send_ack(const struct sock *ssk)
+{
+ return !((1 << inet_sk_state_load(ssk)) &
+ (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE));
+}
+
+static void mptcp_send_ack(struct mptcp_sock *msk)
{
struct mptcp_subflow_context *subflow;
- struct sock *pick = NULL;
mptcp_for_each_subflow(msk, subflow) {
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
- if (force) {
- lock_sock(ssk);
+ lock_sock(ssk);
+ if (tcp_can_send_ack(ssk))
tcp_send_ack(ssk);
- release_sock(ssk);
- continue;
- }
-
- /* if the hintes ssk is still active, use it */
- pick = ssk;
- if (ssk == msk->ack_hint)
- break;
+ release_sock(ssk);
}
- if (!force && pick) {
- lock_sock(pick);
- tcp_cleanup_rbuf(pick, 1);
- release_sock(pick);
+}
+
+static bool mptcp_subflow_cleanup_rbuf(struct sock *ssk)
+{
+ int ret;
+
+ lock_sock(ssk);
+ ret = tcp_can_send_ack(ssk);
+ if (ret)
+ tcp_cleanup_rbuf(ssk, 1);
+ release_sock(ssk);
+ return ret;
+}
+
+static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
+{
+ struct mptcp_subflow_context *subflow;
+
+ /* if the hinted ssk is still active, try to use it */
+ if (likely(msk->ack_hint)) {
+ mptcp_for_each_subflow(msk, subflow) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+ if (msk->ack_hint == ssk &&
+ mptcp_subflow_cleanup_rbuf(ssk))
+ return;
+ }
}
+
+ /* otherwise pick the first active subflow */
+ mptcp_for_each_subflow(msk, subflow)
+ if (mptcp_subflow_cleanup_rbuf(mptcp_subflow_tcp_sock(subflow)))
+ return;
}
static bool mptcp_check_data_fin(struct sock *sk)
@@ -494,7 +520,7 @@ static bool mptcp_check_data_fin(struct sock *sk)
ret = true;
mptcp_set_timeout(sk, NULL);
- mptcp_send_ack(msk, true);
+ mptcp_send_ack(msk);
mptcp_close_wake_up(sk);
}
return ret;
@@ -1579,6 +1605,11 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
return -EOPNOTSUPP;
lock_sock(sk);
+ if (unlikely(sk->sk_state == TCP_LISTEN)) {
+ copied = -ENOTCONN;
+ goto out_err;
+ }
+
timeo = sock_rcvtimeo(sk, nonblock);
len = min_t(size_t, len, INT_MAX);
@@ -1604,7 +1635,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
/* be sure to advertise window change */
old_space = READ_ONCE(msk->old_wspace);
if ((tcp_space(sk) - old_space) >= old_space)
- mptcp_send_ack(msk, false);
+ mptcp_cleanup_rbuf(msk);
/* only the master socket status is relevant here. The exit
* conditions mirror closely tcp_recvmsg()
--
2.26.2
^ permalink raw reply related
* Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink message format
From: Eelco Chaudron @ 2020-11-24 11:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Matteo Croce, netdev, David Miller, dev, Pravin B Shelar,
bindiyakurle, Ilya Maximets
In-Reply-To: <20201123175739.13a27aed@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On 24 Nov 2020, at 2:57, Jakub Kicinski wrote:
> On Mon, 23 Nov 2020 20:36:39 +0100 Matteo Croce wrote:
>> On Fri, Nov 20, 2020 at 10:12 PM Jakub Kicinski <kuba@kernel.org>
>> wrote:
>>> On Thu, 19 Nov 2020 04:04:04 -0500 Eelco Chaudron wrote:
>>>> Currently, the openvswitch module is not accepting the correctly
>>>> formated
>>>> netlink message for the TTL decrement action. For both setting and
>>>> getting
>>>> the dec_ttl action, the actions should be nested in the
>>>> OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h
>>>> uapi.
>>>
>>> IOW this change will not break any known user space, correct?
>>>
>>> But existing OvS user space already expects it to work like you
>>> make it work now?
>>>
>>> What's the harm in leaving it as is?
>>>
>>>> Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>
>>> Can we get a review from OvS folks? Matteo looks good to you (as the
>>> original author)?
>>
>> I think that the userspace still has to implement the dec_ttl action;
>> by now dec_ttl is implemented with set_ttl().
>> So there is no breakage yet.
>>
>> Eelco, with this fix we will encode the netlink attribute in the same
>> way for the kernel and netdev datapath?
>
> We don't allow breaking uAPI. Sounds like the user space never
> implemented this and perhaps the nesting is just inconvenient
> but not necessarily broken? If it is broken and unusable that
> has to be clearly explained in the commit message. I'm dropping
> v1 from patchwork.
Thanks, I will add some explaining comments to the V2, and sent it out.
^ permalink raw reply
* Re: [PATCH 1/3] xsk: replace datagram_poll by sock_poll_wait
From: Magnus Karlsson @ 2020-11-24 11:36 UTC (permalink / raw)
To: Xuan Zhuo
Cc: Björn Töpel, Magnus Karlsson, Jonathan Lemon,
David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Network Development, bpf, linux-kernel
In-Reply-To: <CAJ8uoz3PtqzbfCD6bv1LQOtPVH3qf4mc=V=u_emTxtq3yYUeYw@mail.gmail.com>
On Mon, Nov 23, 2020 at 3:11 PM Magnus Karlsson
<magnus.karlsson@gmail.com> wrote:
>
> On Wed, Nov 18, 2020 at 9:26 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > datagram_poll will judge the current socket status (EPOLLIN, EPOLLOUT)
> > based on the traditional socket information (eg: sk_wmem_alloc), but
> > this does not apply to xsk. So this patch uses sock_poll_wait instead of
> > datagram_poll, and the mask is calculated by xsk_poll.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> > net/xdp/xsk.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > index cfbec39..7f0353e 100644
> > --- a/net/xdp/xsk.c
> > +++ b/net/xdp/xsk.c
> > @@ -477,11 +477,13 @@ static int xsk_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
> > static __poll_t xsk_poll(struct file *file, struct socket *sock,
> > struct poll_table_struct *wait)
> > {
> > - __poll_t mask = datagram_poll(file, sock, wait);
> > + __poll_t mask = 0;
>
> It would indeed be nice to not execute a number of tests in
> datagram_poll that will never be triggered. It will speed up things
> for sure. But we need to make sure that removing those flags that
> datagram_poll sets do not have any bad effects in the code above this.
> But let us tentatively keep this patch for the next version of the
> patch set. Just need to figure out how to solve your problem in a nice
> way first. See discussion in patch 0/3.
>
> > struct sock *sk = sock->sk;
> > struct xdp_sock *xs = xdp_sk(sk);
> > struct xsk_buff_pool *pool;
> >
> > + sock_poll_wait(file, sock, wait);
> > +
> > if (unlikely(!xsk_is_bound(xs)))
> > return mask;
> >
> > --
> > 1.8.3.1
> >
The fix looks correct and it will speed things up too as a bonus.
Please include this patch in the v2 as outlined in my answer to 0/3.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 1/2] dt-bindings: net: nfc: s3fwrn5: Support a UART interface
From: Bongsu Jeon @ 2020-11-24 11:39 UTC (permalink / raw)
To: krzk@kernel.org
Cc: Bongsu Jeon, kuba@kernel.org, linux-nfc@lists.01.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20201123080123.GA5656@kozik-lap>
On Mon, Nov 23, 2020 at 5:02 PM krzk@kernel.org <krzk@kernel.org> wrote:
>
> On Mon, Nov 23, 2020 at 04:55:26PM +0900, Bongsu Jeon wrote:
> > Since S3FWRN82 NFC Chip, The UART interface can be used.
> > S3FWRN82 supports I2C and UART interface.
> >
> > Signed-off-by: Bongsu Jeon <bongsu.jeon@samsung.com>
> > ---
> > .../bindings/net/nfc/samsung,s3fwrn5.yaml | 28 +++++++++++++++++--
> > 1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> > index cb0b8a560282..37b3e5ae5681 100644
> > --- a/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> > +++ b/Documentation/devicetree/bindings/net/nfc/samsung,s3fwrn5.yaml
> > @@ -13,6 +13,7 @@ maintainers:
> > properties:
> > compatible:
> > const: samsung,s3fwrn5-i2c
> > + const: samsung,s3fwrn82-uart
>
> This does not work, you need to use enum. Did you run at least
> dt_bindings_check?
>
Sorry. I didn't. I fixed it as below and ran dt_bindings_check.
compatible:
oneOf:
- enum:
- samsung,s3fwrn5-i2c
- samsung,s3fwrn82
> The compatible should be just "samsung,s3fwrn82". I think it was a
> mistake in the first s3fwrn5 submission to add a interface to
> compatible.
>
Ok. I will change the name.
> >
> > en-gpios:
> > maxItems: 1
> > @@ -47,10 +48,19 @@ additionalProperties: false
> > required:
> > - compatible
> > - en-gpios
> > - - interrupts
> > - - reg
> > - wake-gpios
> >
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: samsung,s3fwrn5-i2c
> > + then:
> > + required:
> > + - interrupts
> > + - reg
> > +
> > examples:
> > - |
> > #include <dt-bindings/gpio/gpio.h>
> > @@ -71,3 +81,17 @@ examples:
> > wake-gpios = <&gpj0 2 GPIO_ACTIVE_HIGH>;
> > };
> > };
> > + # UART example on Raspberry Pi
> > + - |
> > + &uart0 {
> > + status = "okay";
> > +
> > + s3fwrn82_uart {
>
> Just "bluetooth" to follow Devicetree specification.
Sorry. I don't understand this comment.
Could you explain it?
Does it mean i need to refer to the net/broadcom-bluetooth.txt?
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH net-next 1/2] dt-bindings: net: nfc: s3fwrn5: Support a UART interface
From: krzk @ 2020-11-24 11:41 UTC (permalink / raw)
To: Bongsu Jeon
Cc: Bongsu Jeon, kuba@kernel.org, linux-nfc@lists.01.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CACwDmQBh77pqivk=bBv3SJ14HLucY42jZyEaKAX+n=yS3TSqFw@mail.gmail.com>
On Tue, Nov 24, 2020 at 08:39:40PM +0900, Bongsu Jeon wrote:
> On Mon, Nov 23, 2020 at 5:02 PM krzk@kernel.org <krzk@kernel.org> wrote:
> >
> > On Mon, Nov 23, 2020 at 04:55:26PM +0900, Bongsu Jeon wrote:
> > examples:
> > > - |
> > > #include <dt-bindings/gpio/gpio.h>
> > > @@ -71,3 +81,17 @@ examples:
> > > wake-gpios = <&gpj0 2 GPIO_ACTIVE_HIGH>;
> > > };
> > > };
> > > + # UART example on Raspberry Pi
> > > + - |
> > > + &uart0 {
> > > + status = "okay";
> > > +
> > > + s3fwrn82_uart {
> >
> > Just "bluetooth" to follow Devicetree specification.
> Sorry. I don't understand this comment.
> Could you explain it?
> Does it mean i need to refer to the net/broadcom-bluetooth.txt?
The node name should be "bluetooth", not "s3fwrn82_uart", because of
Devicetree naming convention - node names should represent generic class
of a device.
Best regards,
Krzysztof
^ permalink raw reply
* RE: [PATCH net-next v5 3/5] net/lapb: fix t1 timer handling for LAPB_STATE_0
From: David Laight @ 2020-11-24 11:43 UTC (permalink / raw)
To: 'Martin Schiller', andrew.hendry@gmail.com,
davem@davemloft.net, kuba@kernel.org, xie.he.0141@gmail.com
Cc: linux-x25@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20201124093538.21177-4-ms@dev.tdt.de>
From: Martin Schiller
> Sent: 24 November 2020 09:36
>
> 1. DTE interface changes immediately to LAPB_STATE_1 and start sending
> SABM(E).
>
> 2. DCE interface sends N2-times DM and changes to LAPB_STATE_1
> afterwards if there is no response in the meantime.
Seems reasonable.
It is 35 years since I wrote LAPB and I can't exactly remember
what we did.
If I stole a copy of the code it's on a QIC-150 tape cartridge!
I really don't remember having a DTE/DCE option.
It is likely that LAPB came up sending DM (response without F)
until level3 requested the link come up when it would send
N2 SABM+P hoping to get a UA+F.
It would then send DM-F until a retry request was made.
We certainly had several different types of crossover connectors
for DTE-DTE working.
David
>
> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> ---
> net/lapb/lapb_timer.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/lapb/lapb_timer.c b/net/lapb/lapb_timer.c
> index 8f5b17001a07..baa247fe4ed0 100644
> --- a/net/lapb/lapb_timer.c
> +++ b/net/lapb/lapb_timer.c
> @@ -85,11 +85,18 @@ static void lapb_t1timer_expiry(struct timer_list *t)
> switch (lapb->state) {
>
> /*
> - * If we are a DCE, keep going DM .. DM .. DM
> + * If we are a DCE, send DM up to N2 times, then switch to
> + * STATE_1 and send SABM(E).
> */
> case LAPB_STATE_0:
> - if (lapb->mode & LAPB_DCE)
> + if (lapb->mode & LAPB_DCE &&
> + lapb->n2count != lapb->n2) {
> + lapb->n2count++;
> lapb_send_control(lapb, LAPB_DM, LAPB_POLLOFF, LAPB_RESPONSE);
> + } else {
> + lapb->state = LAPB_STATE_1;
> + lapb_establish_data_link(lapb);
> + }
> break;
>
> /*
> --
> 2.20.1
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH v3] brcmfmac: expose firmware config files through modinfo
From: matthias.bgg @ 2020-11-24 12:00 UTC (permalink / raw)
To: Jakub Kicinski, Kalle Valo, David S . Miller, hdegoede
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Matthias Brugger, digetx,
Saravanan Shanmugham, linux-kernel, Ulf Hansson, Amar Shankar,
brcm80211-dev-list
From: Matthias Brugger <mbrugger@suse.com>
Apart from a firmware binary the chip needs a config file used by the
FW. Add the config files to modinfo so that they can be read by
userspace.
Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
Changes in v3:
Use only two more generic wildcards.
Changes in v2:
In comparison to first version [0] we use wildcards to enumerate the
firmware configuration files. Wildcard support was added to dracut
recently [1].
[0] https://lore.kernel.org/linux-wireless/20200701153123.25602-1-matthias.bgg@kernel.org/
[1] https://github.com/dracutdevs/dracut/pull/860
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 99987a789e7e..6fe91c537adf 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -625,6 +625,10 @@ BRCMF_FW_DEF(4359, "brcmfmac4359-sdio");
BRCMF_FW_DEF(4373, "brcmfmac4373-sdio");
BRCMF_FW_DEF(43012, "brcmfmac43012-sdio");
+/* firmware config files */
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac*-sdio.*.txt");
+MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac*-pcie.*.txt");
+
static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
BRCMF_FW_ENTRY(BRCM_CC_43143_CHIP_ID, 0xFFFFFFFF, 43143),
BRCMF_FW_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0000001F, 43241B0),
--
2.29.2
^ permalink raw reply related
* Re: [PATCH v3] brcmfmac: expose firmware config files through modinfo
From: Hans de Goede @ 2020-11-24 12:01 UTC (permalink / raw)
To: matthias.bgg, Jakub Kicinski, Kalle Valo, David S . Miller
Cc: Pali Rohár, Guenter Roeck, Chi-Hsien Lin, Franky Lin,
Chung-Hsien Hsu, Jean-Philippe Brucker, Double Lo, Frank Kao,
linux-wireless, brcm80211-dev-list.pdl, Arend van Spriel,
Gustavo A . R . Silva, netdev, Rafał Miłecki,
Hante Meuleman, Wright Feng, Matthias Brugger, digetx,
Saravanan Shanmugham, linux-kernel, Ulf Hansson, Amar Shankar,
brcm80211-dev-list
In-Reply-To: <20201124120018.31358-1-matthias.bgg@kernel.org>
Hi,
On 11/24/20 1:00 PM, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <mbrugger@suse.com>
>
> Apart from a firmware binary the chip needs a config file used by the
> FW. Add the config files to modinfo so that they can be read by
> userspace.
>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>
> ---
>
> Changes in v3:
> Use only two more generic wildcards.
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
>
> Changes in v2:
> In comparison to first version [0] we use wildcards to enumerate the
> firmware configuration files. Wildcard support was added to dracut
> recently [1].
> [0] https://lore.kernel.org/linux-wireless/20200701153123.25602-1-matthias.bgg@kernel.org/
> [1] https://github.com/dracutdevs/dracut/pull/860
>
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index 99987a789e7e..6fe91c537adf 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -625,6 +625,10 @@ BRCMF_FW_DEF(4359, "brcmfmac4359-sdio");
> BRCMF_FW_DEF(4373, "brcmfmac4373-sdio");
> BRCMF_FW_DEF(43012, "brcmfmac43012-sdio");
>
> +/* firmware config files */
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac*-sdio.*.txt");
> +MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcm/brcmfmac*-pcie.*.txt");
> +
> static const struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
> BRCMF_FW_ENTRY(BRCM_CC_43143_CHIP_ID, 0xFFFFFFFF, 43143),
> BRCMF_FW_ENTRY(BRCM_CC_43241_CHIP_ID, 0x0000001F, 43241B0),
>
^ permalink raw reply
* [PATCH v7 0/3] AX88796C SPI Ethernet Adapter
From: Łukasz Stelmach @ 2020-11-24 12:03 UTC (permalink / raw)
To: Andrew Lunn, jim.cromie, Heiner Kallweit, David S. Miller,
Jakub Kicinski, Rob Herring, Kukjin Kim, Krzysztof Kozlowski,
Russell King, netdev, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc
Cc: Bartłomiej Żolnierkiewicz, Marek Szyprowski,
Łukasz Stelmach
In-Reply-To: <CGME20201124120336eucas1p20710fdff8434428ea0f011b02249b8a8@eucas1p2.samsung.com>
This is a driver for AX88796C Ethernet Adapter connected in SPI mode as
found on ARTIK5 evaluation board. The driver has been ported from a
v3.10.9 vendor kernel for ARTIK5 board.
Changes in v7:
- removed duplicate code
- moved a constant buffer definition away from a header file
Changes in v6:
- fixed typos in Kconfig
- checked argument value in ax88796c_set_tunable
- updated tags in commit messages
Changes in v5:
- coding style (local variable declarations)
- added spi0 node in the DT binding example and removed
interrupt-parent
- removed comp module parameter
- added CONFIG_SPI_AX88796C_COMPRESSION option to set the initial
state of SPI compression
- introduced new ethtool tunable "spi-compression" to controll SPI
transfer compression
- removed unused fields in struct ax88796c_device
- switched from using buffers allocated on stack for SPI transfers
to DMA safe ones embedded in struct ax_spi and allocated with
kmalloc()
Changes in v4:
- fixed compilation problems in asix,ax88796c.yaml and in
ax88796c_main.c introduced in v3
Changes in v3:
- modify vendor-prefixes.yaml in a separate patch
- fix several problems in the dt binding
- removed unnecessary descriptions and properties
- changed the order of entries
- fixed problems with missing defines in the example
- change (1 << N) to BIT(N), left a few (0 << N)
- replace ax88796c_get_link(), ax88796c_get_link_ksettings(),
ax88796c_set_link_ksettings(), ax88796c_nway_reset(),
ax88796c_set_mac_address() with appropriate kernel functions.
- disable PHY auto-polling in MAC and use PHYLIB to track the state
of PHY and configure MAC
- propagate return values instead of returning constants in several
places
- add WARN_ON() for unlocked mutex
- remove local work queue and use the system_wq
- replace phy_connect_direct() with phy_connect() and move
devm_register_netdev() to the end of ax88796c_probe()
(Unlike phy_connect_direct() phy_connect() does not crash if the
network device isn't registered yet.)
- remove error messages on ENOMEM
- move free_irq() to the end of ax88796c_close() to avoid race
condition
- implement flow-control
Changes in v2:
- use phylib
- added DT bindings
- moved #includes to *.c files
- used mutex instead of a semaphore for locking
- renamed some constants
- added error propagation for several functions
- used ethtool for dumping registers
- added control over checksum offloading
- remove vendor specific PM
- removed macaddr module parameter and added support for reading a MAC
address from platform data (e.g. DT)
- removed dependency on SPI from NET_VENDOR_ASIX
- added an entry in the MAINTAINERS file
- simplified logging with appropriate netif_* and netdev_* helpers
- lots of style fixes
Łukasz Stelmach (3):
dt-bindings: vendor-prefixes: Add asix prefix
dt-bindings: net: Add bindings for AX88796C SPI Ethernet Adapter
net: ax88796c: ASIX AX88796C SPI Ethernet Adapter Driver
.../bindings/net/asix,ax88796c.yaml | 73 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 6 +
drivers/net/ethernet/Kconfig | 1 +
drivers/net/ethernet/Makefile | 1 +
drivers/net/ethernet/asix/Kconfig | 35 +
drivers/net/ethernet/asix/Makefile | 6 +
drivers/net/ethernet/asix/ax88796c_ioctl.c | 221 ++++
drivers/net/ethernet/asix/ax88796c_ioctl.h | 26 +
drivers/net/ethernet/asix/ax88796c_main.c | 1132 +++++++++++++++++
drivers/net/ethernet/asix/ax88796c_main.h | 561 ++++++++
drivers/net/ethernet/asix/ax88796c_spi.c | 112 ++
drivers/net/ethernet/asix/ax88796c_spi.h | 69 +
include/uapi/linux/ethtool.h | 1 +
net/ethtool/common.c | 1 +
15 files changed, 2247 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/asix,ax88796c.yaml
create mode 100644 drivers/net/ethernet/asix/Kconfig
create mode 100644 drivers/net/ethernet/asix/Makefile
create mode 100644 drivers/net/ethernet/asix/ax88796c_ioctl.c
create mode 100644 drivers/net/ethernet/asix/ax88796c_ioctl.h
create mode 100644 drivers/net/ethernet/asix/ax88796c_main.c
create mode 100644 drivers/net/ethernet/asix/ax88796c_main.h
create mode 100644 drivers/net/ethernet/asix/ax88796c_spi.c
create mode 100644 drivers/net/ethernet/asix/ax88796c_spi.h
--
2.26.2
^ permalink raw reply
* [PATCH v7 1/3] dt-bindings: vendor-prefixes: Add asix prefix
From: Łukasz Stelmach @ 2020-11-24 12:03 UTC (permalink / raw)
To: Andrew Lunn, jim.cromie, Heiner Kallweit, David S. Miller,
Jakub Kicinski, Rob Herring, Kukjin Kim, Krzysztof Kozlowski,
Russell King, netdev, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc
Cc: Bartłomiej Żolnierkiewicz, Marek Szyprowski,
Łukasz Stelmach, Rob Herring
In-Reply-To: <20201124120330.32445-1-l.stelmach@samsung.com>
Add the prefix for ASIX Electronics Corporation.
Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2735be1a8470..ce3b3f6c9728 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -117,6 +117,8 @@ patternProperties:
description: Asahi Kasei Corp.
"^asc,.*":
description: All Sensors Corporation
+ "^asix,.*":
+ description: ASIX Electronics Corporation
"^aspeed,.*":
description: ASPEED Technology Inc.
"^asus,.*":
--
2.26.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox