* Re: [PATCH net-next 2/5] net: sched: em_ipt: set the family based on the protocol when matching
From: Eyal Birger @ 2019-06-26 13:33 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, roopa, pablo, xiyou.wangcong, davem, jiri, jhs
In-Reply-To: <20190626115855.13241-3-nikolay@cumulusnetworks.com>
Hi Nikolay,
On Wed, 26 Jun 2019 14:58:52 +0300
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> Set the family based on the protocol otherwise protocol-neutral
> matches will have wrong information (e.g. NFPROTO_UNSPEC). In
> preparation for using NFPROTO_UNSPEC xt matches.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> net/sched/em_ipt.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/em_ipt.c b/net/sched/em_ipt.c
> index 64dbafe4e94c..23965a071177 100644
> --- a/net/sched/em_ipt.c
> +++ b/net/sched/em_ipt.c
> @@ -189,10 +189,12 @@ static int em_ipt_match(struct sk_buff *skb,
> struct tcf_ematch *em, case htons(ETH_P_IP):
> if (!pskb_network_may_pull(skb, sizeof(struct
> iphdr))) return 0;
> + state.pf = NFPROTO_IPV4;
> break;
> case htons(ETH_P_IPV6):
> if (!pskb_network_may_pull(skb, sizeof(struct
> ipv6hdr))) return 0;
> + state.pf = NFPROTO_IPV6;
> break;
> default:
> return 0;
> @@ -203,7 +205,7 @@ static int em_ipt_match(struct sk_buff *skb,
> struct tcf_ematch *em, if (skb->skb_iif)
> indev = dev_get_by_index_rcu(em->net, skb->skb_iif);
>
> - nf_hook_state_init(&state, im->hook, im->match->family,
> + nf_hook_state_init(&state, im->hook, state.pf,
> indev ?: skb->dev, skb->dev, NULL,
> em->net, NULL);
> acpar.match = im->match;
I think this change is incompatible with current behavior.
Consider the 'policy' match which matches the packet's xfrm state (sec_path)
with the provided user space parameters. The sec_path includes information
about the encapsulating packet's parameters whereas the current skb points to
the encapsulated packet, and the match is done on the encapsulating
packet's info.
So if you have an IPv6 packet encapsulated within an IPv4 packet, the match
parameters should be done using IPv4 parameters, not IPv6.
Maybe use the packet's family only if the match family is UNSPEC?
Eyal.
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-26 13:36 UTC (permalink / raw)
To: Johannes Berg, Dan Williams, Arnd Bergmann
Cc: Subash Abhinov Kasiviswanathan, abhishek.esse, Ben Chan,
Bjorn Andersson, cpratapa, David Miller, DTML, Eric Caruso,
evgreen, Ilias Apalodimas, Linux ARM, linux-arm-msm,
Linux Kernel Mailing List, linux-soc, Networking, syadagir
In-Reply-To: <868e949b1fc8cf22307f579ab1f14543064bec20.camel@sipsolutions.net>
On 6/25/19 9:14 AM, Johannes Berg wrote:
> Hi Alex,
>
> I'll just pick a few or your messages and reply there - some other
> subthreads seem to have pretty much completed.
>
. . .
>>> Linux usually tries to keep drivers generic and focused; each driver is
>>> written for a specific function. For example, a USB device usually
>>> provides multiple USB interfaces which will be bound to different Linux
>>> drivers like a TTY, cdc-ether, QMI (via qmi_wwan), cdc-acm, etc.
>>
>> So USB has some attributes similar to what we're talking about
>> here. But if I'm not mistaken we want some sort of an overall
>> management scheme as well.
>
> Yes. For the record, I think the part about "keep drivers generic and
> focused" really only works for USB devices that expose different pieces
> that look like any other network device or a TTY device on the USB
> level, just the combination of these things (and knowing about that)
> really makes them a modem.
>
> For things like IPA or the (hypothetical) Intel driver we're talking
> about, it's still all managed by a single (PCIe) driver. For the Intel
> device in particular, even all the control channels are over exactly the
> same transport mechanism as the data channels.
Actually the setup for IPA requires certain things to be done via
QMI by something outside the IPA driver, and it uses a separate
communication path. But I understand what you're saying.
. . .
>> I don't like the "maybe" API unless there's no other way to do it.
>>
>> Instead I think it would be better for the probing driver to register
>> with a whatever the WWAN core is, and then have the WWAN core be
>> responsible for pulling things all together when it receives a
>> request to do so. I.e., something in user space should request
>> that a registered data interface be brought up, and at that
>> time everything "knows" it's implemented as part of a WWAN
>> device.
>
> Right, but then we just punt to userspace. Mostly we *do* (eventually!)
> know that it's a WWAN device, just not every component can detect it.
> Some components typically can.
We need to identify the existence of a WWAN device (which is I
guess--typically? always?--a modem). Perhaps that can be
discovered in some cases but I think it means a node described
by Device Tree.
> So for example, you might have a USB multi-function device with a
> network function (looks just like ethernet pretty much) but another TTY
> control channel that actually has some specific WWAN IDs, so that part
> can know it's a WWAN.
>
> Here, the ethernet function would need "maybe" attach, and the control
> channel would "definitively" attach, pulling it together as a WWAN
> device without requiring any more action or information.
So you're saying you have a single Ethernet driver, and it can
drive an Ethernet device connected to a WWAN, or not connected
to a WWAN, without any changes. The only distinction is that
if the device is part of a WWAN it needs to register with the
WWAN framework. Is that right?
>> So maybe:
>> - Hardware probe detects a WWAN device
>> - The drivers that detect the WWAN device register it with the
>> WWAN core code.
>> - A control channel is instantiated at/before the time the WWAN
>> device is registered
>> - Something in user space should manage the bring-up of any
>> other things on the WWAN device thereafter
>
> But those things need to actually get connected first :-)
What I meant is that the registering with the "WWAN core code"
is what does that "connecting." The WWAN code has the information
about what got registered. But as I said above, this WWAN device
needs to be identified, and I think (at least for IPA) that will
require something in Device Tree. That will "connect" them.
Or I might be misunderstanding your point.
> In IPA/Intel case this is easy since it's a single driver. But if
> there's multi-function device with ethernet being a completely separate
> driver, the control channel cannot even reach that to tell it to create
> a new data channel.
There's a lot more to talk about with control. I think
you discuss this in another message, and I'll get to that
shortly. But I think understand your point, and again
I think it comes back to having an abstraction that
represents the modem, distinct from (but "connected" to)
the functions it implements/includes.
>>> userspace should probably always create the netdevices (since they are
>>> always useless until userspace coordinates with the firmware about
>>> them) but that's not how things are yet.
>>
>> That's too bad. How hard would that be to change?
>
> Depends, but as I said above it's probably orthogonal to the question.
> The data channel driver would still need to attach to the WWAN device
> somehow so it becomes reachable by the control plane (note this isn't
> the same as "control channel" since the latter talks to the modem, the
> control plane talks to the kernel drivers).
>
>>>> - What causes a created channel to be removed?
>>>
>>> Driver removal, userspace WWAN daemon terminating the packet data
>>> connection which the channel represents, the modem terminating the
>>> packet data connection (eg network initiated disconnect), etc.
>>
>> OK this is as I expected. Driver (or device) removal is somewhat
>> obvious, but you're confirming user space might request it as well.
>
> If userspace actually had the ability to create (data) channels, then it
> would have the ability to also remove them. Right now, this may or may
> not be supported by the drivers that act together to form the interfaces
> to a WWAN device.
I think this (user space control) needs to be an option, but
it doesn't have to be the only way.
. . .
You made some other good clarifications in this message but I'm
going to try to capture them elsewhere rather than respond here.
-Alex
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-26 13:39 UTC (permalink / raw)
To: Johannes Berg, Arnd Bergmann
Cc: Dan Williams, Subash Abhinov Kasiviswanathan, abhishek.esse,
Ben Chan, Bjorn Andersson, cpratapa, David Miller, DTML,
Eric Caruso, evgreen, Ilias Apalodimas, Linux ARM, linux-arm-msm,
Linux Kernel Mailing List, linux-soc, Networking, syadagir
In-Reply-To: <efbcb3b84ff0a7d7eab875c37f3a5fa77e21d324.camel@sipsolutions.net>
On 6/25/19 9:19 AM, Johannes Berg wrote:
> On Mon, 2019-06-24 at 18:40 +0200, Arnd Bergmann wrote:
>> On Mon, Jun 24, 2019 at 6:21 PM Alex Elder <elder@linaro.org> wrote:
>>> On 6/18/19 2:03 PM, Johannes Berg wrote:
>>>
>>>> Really there are two possible ways (and they intersect to some extent).
>>>>
>>>> One is the whole multi-function device, where a single WWAN device is
>>>> composed of channels offered by actually different drivers, e.g. for a
>>>> typical USB device you might have something like cdc_ether and the
>>>> usb_wwan TTY driver. In this way, we need to "compose" the WWAN device
>>>> similarly, e.g. by using the underlying USB device "struct device"
>>>> pointer to tie it together.
>>>
>>> I *think* this model makes the most sense. But at this point
>>> it would take very little to convince me otherwise... (And then
>>> I saw Arnd's message advocating the other one, unfortunately...)
>>>
>>>> The other is something like IPA or the Intel modem driver, where the
>>>> device is actually a single (e.g. PCIe) device and just has a single
>>>> driver, but that single driver offers different channels.
>>>
>>> What I don't like about this is that it's more monolithic. It
>>> seems better to have the low-level IPA or Intel modem driver (or
>>> any other driver that can support communication between the AP
>>> and WWAN device) present communication paths that other function-
>>> specific drivers can attach to and use.
>>
>> I did not understand Johannes description as two competing models
>> for the same code, but rather two kinds of existing hardware that
>> a new driver system would have to deal with.
>
> Right.
>
>> I was trying to simplify it to just having the second model, by adding
>> a hack to support the first, but my view was rather unpopular so
>> far, so if everyone agrees on one way to do it, don't worry about me ;-)
>
> :-)
>
> However, to also reply to Alex: I don't know exactly how IPA works, but
> for the Intel modem at least you can't fundamentally have two drivers
> for different parts of the functionality, since it's just a single piece
> of hardware and you need to allocate hardware resources from a common
> pool etc. So you cannot split the driver into "Intel modem control
> channel driver" and "Intel modem data channel driver". In fact, it's
> just a single "struct device" on the PCIe bus that you can bind to, and
> only one driver can bind at a time.
Interesting. So a single modem driver needs to implement
*all* of the features/functions? Like GPS or data log or
whatever, all needs to share the same struct device?
Or does what you're describing apply to a subset of the
modem's functionality? Or something else?
> So, IOW, I'm not sure I see how you'd split that up. I guess you could
> if you actually do something like the "rmnet" model, and I suppose
> you're free to do that for IPA if you like, but I tend to think that's
> actually a burden, not a win since you just get more complex code that
> needs to interact with more pieces. A single driver for a single
> hardware that knows about the few types of channels seems simpler to me.
>
>> - to answer Johannes question, my understanding is that the interface
>> between kernel and firmware/hardware for IPA has a single 'struct
>> device' that is used for both the data and the control channels,
>> rather than having a data channel and an independent control device,
>> so this falls into the same category as the Intel one (please correct
>> me on that)
I don't think that's quite right, but it might be partially
right. There is a single device representing IPA, but the
picture is a little more complicated.
The IPA hardware is actually something that sits *between* the
AP and the modem. It implements one form of communication
pathway (IP data), but there are others (including QMI, which
presents a network-like interface but it's actually implemented
via clever use of shared memory and interrupts).
What we're talking about here is WWAN/modem management more
generally though. It *sounds* like the Intel modem is
more like a single device, which requires a single driver,
that seems to implement a bunch of distinct functions.
On this I'm not very knowledgeable but for Qualcomm there is
user space code that is in charge of overall management of
the modem. It implements what I think you're calling control
functions, negotiating with the modem to allow new data channels
to be created. Normally the IPA driver would provide information
to user space about available resources, but would only make a
communication pathway available when requested.
I'm going to leave it at that for now.
> That sounds about the same then, right.
>
> Are the control channels to IPA are actually also tunnelled over the
> rmnet protocol? And even if they are, perhaps they have a different
> hardware queue or so? That'd be the case for Intel - different hardware
> queue, same (or at least similar) protocol spoken for the DMA hardware
> itself, but different contents of the messages obviously.
I want to be careful talking about "control" but for IPA it comes
from user space. For the purpose of getting initial code upstream,
all of that control functionality (which was IOCTL based) has been
removed, and a fixed configuration is assumed.
>> - The user space being proprietary is exactly what we need to avoid
>> with the wwan subsystem. We need to be able to use the same
>> method for setting up Intel, Qualcomm, Samsung, Unisoc or
>> Hisilicon modems or anything else that hooks into the subsystem,
>> and support that in network manager as well as the Android
>> equivalent.
>> If Qualcomm wants to provide their own proprietary user space
>> solution, we can't stop them, but then that should also work on
>> all the others unless they intentionally break it. ;-)
I won't comment on this, in part because I really don't know
right now what is proprietary or why. I think that having
user space (proprietary or not) be able to provide management
capability is a good thing. If a unified kernel interface
provides a common/generic way to manage the modem, I don't
know why Qualcomm wouldn't adapt their code to use it.
But I can't really speak for Qualcomm.
. . .
-Alex
^ permalink raw reply
* Re: WWAN Controller Framework (was IPA [PATCH v2 00/17])
From: Alex Elder @ 2019-06-26 13:40 UTC (permalink / raw)
To: Johannes Berg, davem, arnd, bjorn.andersson, ilias.apalodimas,
Dan Williams
Cc: evgreen, benchan, ejcaruso, cpratapa, syadagir, subashab,
abhishek.esse, netdev, devicetree, linux-kernel, linux-soc,
linux-arm-kernel, linux-arm-msm
In-Reply-To: <f1243295f088b70d48e4b832a28f79c0cd84ca1c.camel@sipsolutions.net>
On 6/25/19 9:34 AM, Johannes Berg wrote:
> On Mon, 2019-06-24 at 12:06 -0500, Alex Elder wrote:
>
>>> OK I want to try to organize a little more concisely some of the
>>> discussion on this, because there is a very large amount of volume
>>> to date and I think we need to try to narrow the focus back down
>>> again.
>
> Sounds good to me!
. . .
>>> - A WWAN unit shall implement a *WWAN control function*, used to
>>> manage the use of other WWAN functions, as well as the WWAN unit
>>> itself.
>
> I think here we need to be more careful. I don't know how you want to
> call it, but we actually have multiple levels of control here.
I completely agree with you. From what I understand there exists
a control channel (or even more than one?) that serves a very
specific purpose in modem management. The main reason I mention
the WWAN control function is that someone (maybe you) indicated
that a control channel automatically gets created.
But I agree, we need to be careful to avoid confusion here.
> You have
> * hardware control, to control how you actually use the (multiple or
> not) physical communication channel(s) to the WWAN unit
> * this is partially exposed to userspace via the WWAN netlink family or
> something like that, so userspace can create new netdevs to tx/rx
> with the "data function" and to the network; note that it could be
> one or multiple
> * WWAN control, which is typically userspace communicating with the
> WWAN control function in the WWAN unit, but this can take different
> forms (as I mentioned earlier, e.g. AT commands, MBIM, QMI)
>
>>> - The AP communicates with a WWAN function using a WWAN protocol.
>
> Right, that's just device specific (IPA vs. Intel vs. ...)
>
>>> - A WWAN physical channel can be *multiplexed*, in which case it
>>> carries the data for one or more *WWAN logical channels*.
>
> This ... depends a bit on how you exactly define a physical channel
> here. Is that, to you, the PCIe/USB link? In that case, yes, obviously
> you have only one physical channel for each WWAN unit.
I think that was what I was trying to capture. There exists
one or more "physical" communication paths between the AP
and WWAN unit/modem. And while one path *could* carry just
one type of traffic, it could also carry multiple logical
channels of traffic by multiplexing.
> However, I'd probably see this slightly differently, because e.g. the
> Intel modem has multiple DMA engines, and so you actually have multiple
> DMA rings to talk to the WWAN unit, and I'd have called each DMA ring a
> physical channel. And then, you just have a 1:1 from physical to logical
> channel since it doesn't actually carry a multiplexing protocol.
Understood.
. . .
> I only disagree slightly on the control planes (there are multiple, and
> multiple options for the "Control function" one), and on the whole
> notion of physical link/logical link/multiplexing which is device
> specific.
>
>>> And if I understand it right, the purpose of the generic framework
>>> being discussed is to define a common mechanism for managing (i.e.,
>>> discovering, creating, destroying, querying, configuring, enabling,
>>> disabling, etc.) WWAN units and the functions they implement, along
>>> with the communication and logical channels used to communicate with
>>> them.
>
> Well, some subset of that matrix, the framework won't actually destroy
> WWAN units I hope ;-)
Hardware self-destruct would be an optional behavior.
> But yes. I'd probably captured it in layers, and say that we have a
>
> WWAN framework layer
> - discover, query, configure WWAN units
> - enable, disable channels to the functions inside the WWAN units
>
> WWAN device driver
> - implement (partial) API offered by WWAN framework layer to allow
> these things
> (sometimes may not allow creating more control or data channels for
> example, and fixed function channels are precreated, but then can
> still discover data about the device and configure the channels
> - implement the device-specific protocols etc. necessary to achieve
> this
>
> Userspace
> - uses control function channel (e.g. TTY) to talk directly to the WWAN
> unit's control function
> - uses WWAN framework APIs to create/configure/... (other) function
> channels
> (it may be necessary to create a control channel even, before being
> able to use it, since different options (AT/MBIM/QMI) may be there
> - configures netdevs (data function channels) after their creation
I don't think I have any argument with this. I'm going to try to
put together something that goes beyond what I wrote in this message,
to try to capture what I think we agree on in a sort of loose design
document.
Thanks Johannes.
-Alex
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Lorenz Bauer @ 2019-06-26 13:45 UTC (permalink / raw)
To: Song Liu
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <20190625182303.874270-2-songliubraving@fb.com>
On Tue, 25 Jun 2019 at 19:23, Song Liu <songliubraving@fb.com> wrote:
>
> This patch introduce unprivileged BPF access. The access control is
> achieved via device /dev/bpf. Users with access to /dev/bpf are able
> to access BPF syscall.
>
> Two ioctl command are added to /dev/bpf:
>
> The first two commands get/put permission to access sys_bpf. This
> permission is noted by setting bit TASK_BPF_FLAG_PERMITTED of
> current->bpf_flags. This permission cannot be inherited via fork().
I know nothing about the scheduler, so pardon my ignorance. Does
TASK_BPF_FLAG_PERMITTED apply per user-space process, or per thread?
--
Lorenz Bauer | Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
www.cloudflare.com
^ permalink raw reply
* Re: [PATCH net-next 2/5] net: sched: em_ipt: set the family based on the protocol when matching
From: Nikolay Aleksandrov @ 2019-06-26 13:45 UTC (permalink / raw)
To: Eyal Birger; +Cc: netdev, roopa, pablo, xiyou.wangcong, davem, jiri, jhs
In-Reply-To: <20190626163353.6d5535cb@jimi>
On 26/06/2019 16:33, Eyal Birger wrote:
> Hi Nikolay,
>
> On Wed, 26 Jun 2019 14:58:52 +0300
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>> Set the family based on the protocol otherwise protocol-neutral
>> matches will have wrong information (e.g. NFPROTO_UNSPEC). In
>> preparation for using NFPROTO_UNSPEC xt matches.
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> net/sched/em_ipt.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/sched/em_ipt.c b/net/sched/em_ipt.c
>> index 64dbafe4e94c..23965a071177 100644
>> --- a/net/sched/em_ipt.c
>> +++ b/net/sched/em_ipt.c
>> @@ -189,10 +189,12 @@ static int em_ipt_match(struct sk_buff *skb,
>> struct tcf_ematch *em, case htons(ETH_P_IP):
>> if (!pskb_network_may_pull(skb, sizeof(struct
>> iphdr))) return 0;
>> + state.pf = NFPROTO_IPV4;
>> break;
>> case htons(ETH_P_IPV6):
>> if (!pskb_network_may_pull(skb, sizeof(struct
>> ipv6hdr))) return 0;
>> + state.pf = NFPROTO_IPV6;
>> break;
>> default:
>> return 0;
>> @@ -203,7 +205,7 @@ static int em_ipt_match(struct sk_buff *skb,
>> struct tcf_ematch *em, if (skb->skb_iif)
>> indev = dev_get_by_index_rcu(em->net, skb->skb_iif);
>>
>> - nf_hook_state_init(&state, im->hook, im->match->family,
>> + nf_hook_state_init(&state, im->hook, state.pf,
>> indev ?: skb->dev, skb->dev, NULL,
>> em->net, NULL);
>> acpar.match = im->match;
>
> I think this change is incompatible with current behavior.
>
> Consider the 'policy' match which matches the packet's xfrm state (sec_path)
> with the provided user space parameters. The sec_path includes information
> about the encapsulating packet's parameters whereas the current skb points to
> the encapsulated packet, and the match is done on the encapsulating
> packet's info.
>
> So if you have an IPv6 packet encapsulated within an IPv4 packet, the match
> parameters should be done using IPv4 parameters, not IPv6.
>
> Maybe use the packet's family only if the match family is UNSPEC?
>
> Eyal.
>
Hi Eyal,
I see your point, I was wondering about the xfrm cases. :)
In such case I think we can simplify the set and do it only on UNSPEC matches as you suggest.
Maybe we should enforce the tc protocol based on the user-specified nfproto at least from
iproute2 otherwise people can add mismatching rules (e.g. nfproto == v6, tc proto == v4).
Thanks,
Nik
^ permalink raw reply
* [PATCH net-next 05/10] net: stmmac: Add the missing speeds that XGMAC supports
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
XGMAC supports following speeds:
- 10G XGMII
- 5G XGMII
- 2.5G XGMII
- 2.5G GMII
- 1G GMII
- 100M MII
- 10M MII
Add them to the stmmac driver.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 6 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 10 ++-
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 ++--
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 79 ++++++++++++++++++----
4 files changed, 86 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 14d8f6d7cb9a..9e8f4dbdcc22 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -392,8 +392,12 @@ struct mac_link {
u32 speed100;
u32 speed1000;
u32 speed2500;
- u32 speed10000;
u32 duplex;
+ struct {
+ u32 speed2500;
+ u32 speed5000;
+ u32 speed10000;
+ } xgmii;
};
struct mii_regs {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 50b41ecb0325..9077b54cbfac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -15,10 +15,14 @@
/* MAC Registers */
#define XGMAC_TX_CONFIG 0x00000000
#define XGMAC_CONFIG_SS_OFF 29
-#define XGMAC_CONFIG_SS_MASK GENMASK(30, 29)
+#define XGMAC_CONFIG_SS_MASK GENMASK(31, 29)
#define XGMAC_CONFIG_SS_10000 (0x0 << XGMAC_CONFIG_SS_OFF)
-#define XGMAC_CONFIG_SS_2500 (0x2 << XGMAC_CONFIG_SS_OFF)
-#define XGMAC_CONFIG_SS_1000 (0x3 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_2500_GMII (0x2 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_1000_GMII (0x3 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_100_MII (0x4 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_5000 (0x5 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_2500 (0x6 << XGMAC_CONFIG_SS_OFF)
+#define XGMAC_CONFIG_SS_10_MII (0x7 << XGMAC_CONFIG_SS_OFF)
#define XGMAC_CONFIG_SARC GENMASK(22, 20)
#define XGMAC_CONFIG_SARC_SHIFT 20
#define XGMAC_CONFIG_JD BIT(16)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index bfa7d6913fd4..0a32c96a7854 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -36,7 +36,7 @@ static void dwxgmac2_core_init(struct mac_device_info *hw,
switch (hw->ps) {
case SPEED_10000:
- tx |= hw->link.speed10000;
+ tx |= hw->link.xgmii.speed10000;
break;
case SPEED_2500:
tx |= hw->link.speed2500;
@@ -381,11 +381,13 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
mac->link.duplex = 0;
- mac->link.speed10 = 0;
- mac->link.speed100 = 0;
- mac->link.speed1000 = XGMAC_CONFIG_SS_1000;
- mac->link.speed2500 = XGMAC_CONFIG_SS_2500;
- mac->link.speed10000 = XGMAC_CONFIG_SS_10000;
+ mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
+ mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
+ mac->link.speed1000 = XGMAC_CONFIG_SS_1000_GMII;
+ mac->link.speed2500 = XGMAC_CONFIG_SS_2500_GMII;
+ mac->link.xgmii.speed2500 = XGMAC_CONFIG_SS_2500;
+ mac->link.xgmii.speed5000 = XGMAC_CONFIG_SS_5000;
+ mac->link.xgmii.speed10000 = XGMAC_CONFIG_SS_10000;
mac->link.speed_mask = XGMAC_CONFIG_SS_MASK;
mac->mii.addr = XGMAC_MDIO_ADDR;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index dc57a2c0a630..1247066c371c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -802,14 +802,43 @@ static void stmmac_validate(struct phylink_config *config,
struct phylink_link_state *state)
{
struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(mac_supported) = { 0, };
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
int tx_cnt = priv->plat->tx_queues_to_use;
int max_speed = priv->plat->max_speed;
+ phylink_set(mac_supported, 10baseT_Half);
+ phylink_set(mac_supported, 10baseT_Full);
+ phylink_set(mac_supported, 100baseT_Half);
+ phylink_set(mac_supported, 100baseT_Full);
+
+ phylink_set(mac_supported, Autoneg);
+ phylink_set(mac_supported, Pause);
+ phylink_set(mac_supported, Asym_Pause);
+ phylink_set_port_modes(mac_supported);
+
+ if (priv->plat->has_gmac ||
+ priv->plat->has_gmac4 ||
+ priv->plat->has_xgmac) {
+ phylink_set(mac_supported, 1000baseT_Half);
+ phylink_set(mac_supported, 1000baseT_Full);
+ phylink_set(mac_supported, 1000baseKX_Full);
+ }
+
/* Cut down 1G if asked to */
if ((max_speed > 0) && (max_speed < 1000)) {
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
+ } else if (priv->plat->has_xgmac) {
+ phylink_set(mac_supported, 2500baseT_Full);
+ phylink_set(mac_supported, 5000baseT_Full);
+ phylink_set(mac_supported, 10000baseSR_Full);
+ phylink_set(mac_supported, 10000baseLR_Full);
+ phylink_set(mac_supported, 10000baseER_Full);
+ phylink_set(mac_supported, 10000baseLRM_Full);
+ phylink_set(mac_supported, 10000baseT_Full);
+ phylink_set(mac_supported, 10000baseKX4_Full);
+ phylink_set(mac_supported, 10000baseKR_Full);
}
/* Half-Duplex can only work with single queue */
@@ -819,7 +848,12 @@ static void stmmac_validate(struct phylink_config *config,
phylink_set(mask, 1000baseT_Half);
}
- bitmap_andnot(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(supported, supported, mac_supported,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_andnot(supported, supported, mask,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
+ bitmap_and(state->advertising, state->advertising, mac_supported,
+ __ETHTOOL_LINK_MODE_MASK_NBITS);
bitmap_andnot(state->advertising, state->advertising, mask,
__ETHTOOL_LINK_MODE_MASK_NBITS);
}
@@ -839,18 +873,37 @@ static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,
ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
ctrl &= ~priv->hw->link.speed_mask;
- switch (state->speed) {
- case SPEED_1000:
- ctrl |= priv->hw->link.speed1000;
- break;
- case SPEED_100:
- ctrl |= priv->hw->link.speed100;
- break;
- case SPEED_10:
- ctrl |= priv->hw->link.speed10;
- break;
- default:
- return;
+ if (state->interface == PHY_INTERFACE_MODE_USXGMII) {
+ switch (state->speed) {
+ case SPEED_10000:
+ ctrl |= priv->hw->link.xgmii.speed10000;
+ break;
+ case SPEED_5000:
+ ctrl |= priv->hw->link.xgmii.speed5000;
+ break;
+ case SPEED_2500:
+ ctrl |= priv->hw->link.xgmii.speed2500;
+ break;
+ default:
+ return;
+ }
+ } else {
+ switch (state->speed) {
+ case SPEED_2500:
+ ctrl |= priv->hw->link.speed2500;
+ break;
+ case SPEED_1000:
+ ctrl |= priv->hw->link.speed1000;
+ break;
+ case SPEED_100:
+ ctrl |= priv->hw->link.speed100;
+ break;
+ case SPEED_10:
+ ctrl |= priv->hw->link.speed10;
+ break;
+ default:
+ return;
+ }
}
priv->speed = state->speed;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 07/10] net: stmmac: Enable support for > 32 Bits addressing in XGMAC
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Currently, stmmac only supports 32 bits addressing for SKB. Enable the
support for upto 48 bits addressing in XGMAC core.
This avoids the use of bounce buffers and increases performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +
.../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 19 ++++++++-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 46 ++++++++++++++++------
5 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 9e8f4dbdcc22..2403a65167b2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -351,6 +351,7 @@ struct dma_features {
unsigned int frpsel;
unsigned int frpbs;
unsigned int frpes;
+ unsigned int addr64;
};
/* GMAC TX FIFO is 8K, Rx FIFO is 16K */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 9077b54cbfac..49b0d945052c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -87,6 +87,7 @@
#define XGMAC_HWFEAT_GMIISEL BIT(1)
#define XGMAC_HW_FEATURE1 0x00000120
#define XGMAC_HWFEAT_TSOEN BIT(18)
+#define XGMAC_HWFEAT_ADDR64 GENMASK(15, 14)
#define XGMAC_HWFEAT_TXFIFOSIZE GENMASK(10, 6)
#define XGMAC_HWFEAT_RXFIFOSIZE GENMASK(4, 0)
#define XGMAC_HW_FEATURE2 0x00000124
@@ -172,6 +173,7 @@
#define XGMAC_EN_LPI BIT(15)
#define XGMAC_LPI_XIT_PKT BIT(14)
#define XGMAC_AAL BIT(12)
+#define XGMAC_EAME BIT(11)
#define XGMAC_BLEN GENMASK(7, 1)
#define XGMAC_BLEN256 BIT(7)
#define XGMAC_BLEN128 BIT(6)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 98fa471da7c0..c4c45402b8f8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -242,8 +242,8 @@ static void dwxgmac2_get_addr(struct dma_desc *p, unsigned int *addr)
static void dwxgmac2_set_addr(struct dma_desc *p, dma_addr_t addr)
{
- p->des0 = cpu_to_le32(addr);
- p->des1 = 0;
+ p->des0 = cpu_to_le32(lower_32_bits(addr));
+ p->des1 = cpu_to_le32(upper_32_bits(addr));
}
static void dwxgmac2_clear(struct dma_desc *p)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 3f84271da836..08f3ed675a39 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -27,7 +27,7 @@ static void dwxgmac2_dma_init(void __iomem *ioaddr,
if (dma_cfg->aal)
value |= XGMAC_AAL;
- writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ writel(value | XGMAC_EAME, ioaddr + XGMAC_DMA_SYSBUS_MODE);
}
static void dwxgmac2_dma_init_chan(void __iomem *ioaddr,
@@ -361,6 +361,23 @@ static void dwxgmac2_get_hw_feature(void __iomem *ioaddr,
/* MAC HW feature 1 */
hw_cap = readl(ioaddr + XGMAC_HW_FEATURE1);
dma_cap->tsoen = (hw_cap & XGMAC_HWFEAT_TSOEN) >> 18;
+
+ dma_cap->addr64 = (hw_cap & XGMAC_HWFEAT_ADDR64) >> 14;
+ switch (dma_cap->addr64) {
+ case 0:
+ dma_cap->addr64 = 32;
+ break;
+ case 1:
+ dma_cap->addr64 = 40;
+ break;
+ case 2:
+ dma_cap->addr64 = 48;
+ break;
+ default:
+ dma_cap->addr64 = 32;
+ break;
+ }
+
dma_cap->tx_fifo_size =
128 << ((hw_cap & XGMAC_HWFEAT_TXFIFOSIZE) >> 6);
dma_cap->rx_fifo_size =
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3a6fac22854e..65ff2a2a15e4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2768,7 +2768,7 @@ static int stmmac_release(struct net_device *dev)
* This function fills descriptor and request new descriptors according to
* buffer length to fill
*/
-static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
+static void stmmac_tso_allocator(struct stmmac_priv *priv, dma_addr_t des,
int total_len, bool last_segment, u32 queue)
{
struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
@@ -2779,11 +2779,18 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
tmp_len = total_len;
while (tmp_len > 0) {
+ dma_addr_t curr_addr;
+
tx_q->cur_tx = STMMAC_GET_ENTRY(tx_q->cur_tx, DMA_TX_SIZE);
WARN_ON(tx_q->tx_skbuff[tx_q->cur_tx]);
desc = tx_q->dma_tx + tx_q->cur_tx;
- desc->des0 = cpu_to_le32(des + (total_len - tmp_len));
+ curr_addr = des + (total_len - tmp_len);
+ if (priv->dma_cap.addr64 <= 32)
+ desc->des0 = cpu_to_le32(curr_addr);
+ else
+ stmmac_set_desc_addr(priv, desc, curr_addr);
+
buff_size = tmp_len >= TSO_MAX_BUFF_SIZE ?
TSO_MAX_BUFF_SIZE : tmp_len;
@@ -2829,11 +2836,12 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
struct stmmac_priv *priv = netdev_priv(dev);
int nfrags = skb_shinfo(skb)->nr_frags;
u32 queue = skb_get_queue_mapping(skb);
- unsigned int first_entry, des;
+ unsigned int first_entry;
struct stmmac_tx_queue *tx_q;
int tmp_pay_len = 0;
u32 pay_len, mss;
u8 proto_hdr_len;
+ dma_addr_t des;
int i;
tx_q = &priv->tx_queue[queue];
@@ -2890,14 +2898,19 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
tx_q->tx_skbuff_dma[first_entry].buf = des;
tx_q->tx_skbuff_dma[first_entry].len = skb_headlen(skb);
- first->des0 = cpu_to_le32(des);
+ if (priv->dma_cap.addr64 <= 32) {
+ first->des0 = cpu_to_le32(des);
- /* Fill start of payload in buff2 of first descriptor */
- if (pay_len)
- first->des1 = cpu_to_le32(des + proto_hdr_len);
+ /* Fill start of payload in buff2 of first descriptor */
+ if (pay_len)
+ first->des1 = cpu_to_le32(des + proto_hdr_len);
- /* If needed take extra descriptors to fill the remaining payload */
- tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
+ /* If needed take extra descriptors to fill the remaining payload */
+ tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
+ } else {
+ stmmac_set_desc_addr(priv, first, des);
+ tmp_pay_len = pay_len;
+ }
stmmac_tso_allocator(priv, des, tmp_pay_len, (nfrags == 0), queue);
@@ -3024,12 +3037,12 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
int i, csum_insertion = 0, is_jumbo = 0;
u32 queue = skb_get_queue_mapping(skb);
int nfrags = skb_shinfo(skb)->nr_frags;
- int entry;
- unsigned int first_entry;
struct dma_desc *desc, *first;
struct stmmac_tx_queue *tx_q;
+ unsigned int first_entry;
unsigned int enh_desc;
- unsigned int des;
+ dma_addr_t des;
+ int entry;
tx_q = &priv->tx_queue[queue];
@@ -4306,6 +4319,15 @@ int stmmac_dvr_probe(struct device *device,
priv->tso = true;
dev_info(priv->device, "TSO feature enabled\n");
}
+
+ if (priv->dma_cap.addr64) {
+ ret = dma_set_mask_and_coherent(device,
+ DMA_BIT_MASK(priv->dma_cap.addr64));
+ if (!ret)
+ dev_info(priv->device, "Using %d bits DMA width\n",
+ priv->dma_cap.addr64);
+ }
+
ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
#ifdef STMMAC_VLAN_TAG_USED
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 09/10] net: stmmac: Only disable interrupts if NAPI is scheduled
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Only disable the interrupts if RX NAPI gets to be scheduled. Also,
schedule the TX NAPI only when the interrupts are disabled.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0893af8b432d..bc949665c529 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2049,12 +2049,12 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
&priv->xstats, chan);
struct stmmac_channel *ch = &priv->channel[chan];
- if (status)
- status |= handle_rx | handle_tx;
-
if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
- stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
- napi_schedule_irqoff(&ch->rx_napi);
+ if (napi_schedule_prep(&ch->rx_napi)) {
+ stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+ __napi_schedule_irqoff(&ch->rx_napi);
+ status |= handle_tx;
+ }
}
if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use))
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 04/10] net: stmmac: dwxgmac: Fix the undefined burst setting
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Undefined burst shall only be set if pdata asks to.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index a1ad49680c07..3f84271da836 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -91,11 +91,11 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
value |= (axi->axi_rd_osr_lmt << XGMAC_RD_OSR_LMT_SHIFT) &
XGMAC_RD_OSR_LMT;
+ if (!axi->axi_fb)
+ value |= XGMAC_UNDEF;
+
value &= ~XGMAC_BLEN;
for (i = 0; i < AXI_BLEN; i++) {
- if (axi->axi_blen[i])
- value &= ~XGMAC_UNDEF;
-
switch (axi->axi_blen[i]) {
case 256:
value |= XGMAC_BLEN256;
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 06/10] net: stmmac: Do not disable interrupts when cleaning TX
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
This is a performance killer and anyways the interrupts are being
disabled by RX NAPI so no need to disable them again.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1247066c371c..3a6fac22854e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2057,10 +2057,8 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
napi_schedule_irqoff(&ch->rx_napi);
}
- if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
- stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+ if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use))
napi_schedule_irqoff(&ch->tx_napi);
- }
return status;
}
@@ -3560,8 +3558,8 @@ static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget)
work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan);
work_done = min(work_done, budget);
- if (work_done < budget && napi_complete_done(napi, work_done))
- stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
+ if (work_done < budget)
+ napi_complete_done(napi, work_done);
/* Force transmission restart */
tx_q = &priv->tx_queue[chan];
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 02/10] net: stmmac: Do not try to enable PHY EEE if MAC does not support it
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Do not enable EEE feature in the PHY if MAC does not support it.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f4593d2d9d20..fab65f129207 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -893,7 +893,7 @@ static void stmmac_mac_link_up(struct phylink_config *config,
struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
stmmac_mac_set(priv, priv->ioaddr, true);
- if (phy) {
+ if (phy && priv->dma_cap.eee) {
priv->eee_active = phy_init_eee(phy, 1) >= 0;
priv->eee_enabled = stmmac_eee_init(priv);
stmmac_set_eee_pls(priv, priv->hw, true);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 01/10] net: stmmac: dwxgmac: Enable EDMA by default
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Enable the EDMA feature by default which gives higher performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index b8296eb41011..50b41ecb0325 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -177,6 +177,8 @@
#define XGMAC_BLEN8 BIT(2)
#define XGMAC_BLEN4 BIT(1)
#define XGMAC_UNDEF BIT(0)
+#define XGMAC_TX_EDMA_CTRL 0x00003040
+#define XGMAC_RX_EDMA_CTRL 0x00003044
#define XGMAC_DMA_CH_CONTROL(x) (0x00003100 + (0x80 * (x)))
#define XGMAC_PBLx8 BIT(16)
#define XGMAC_DMA_CH_TX_CONTROL(x) (0x00003104 + (0x80 * (x)))
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7861a938420a..a1ad49680c07 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -122,6 +122,8 @@ static void dwxgmac2_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
}
writel(value, ioaddr + XGMAC_DMA_SYSBUS_MODE);
+ writel(GENMASK(29, 0), ioaddr + XGMAC_TX_EDMA_CTRL);
+ writel(GENMASK(29, 0), ioaddr + XGMAC_RX_EDMA_CTRL);
}
static void dwxgmac2_dma_rx_mode(void __iomem *ioaddr, int mode,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 00/10] net: stmmac: 10GbE using XGMAC
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
Support for 10Gb Link using XGMAC core plus some performance tweaks.
Tested in a PCI based setup.
iperf3 TCP results:
TSO ON, MTU=1500, TX Queues = 1, RX Queues = 1, Flow Control ON
Pinned CPU (-A), Zero-Copy (-Z)
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec 1 sender
[ 5] 0.00-600.00 sec 643 GBytes 9.21 Gbits/sec receiver
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Jose Abreu (10):
net: stmmac: dwxgmac: Enable EDMA by default
net: stmmac: Do not try to enable PHY EEE if MAC does not support it
net: stmmac: Decrease default RX Watchdog value
net: stmmac: dwxgmac: Fix the undefined burst setting
net: stmmac: Add the missing speeds that XGMAC supports
net: stmmac: Do not disable interrupts when cleaning TX
net: stmmac: Enable support for > 32 Bits addressing in XGMAC
net: stmmac: Update RX Tail Pointer to last free entry
net: stmmac: Only disable interrupts if NAPI is scheduled
net: stmmac: Try to get C45 PHY if everything else fails
drivers/net/ethernet/stmicro/stmmac/common.h | 9 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 14 +-
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 14 +-
.../net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 27 +++-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 165 ++++++++++++++++-----
6 files changed, 178 insertions(+), 55 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next 03/10] net: stmmac: Decrease default RX Watchdog value
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
For performance reasons decrease the default RX Watchdog value for the
minimum allowed.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index ad9e9368535d..14d8f6d7cb9a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -246,7 +246,7 @@ struct stmmac_safety_stats {
/* Max/Min RI Watchdog Timer count value */
#define MAX_DMA_RIWT 0xff
-#define MIN_DMA_RIWT 0x20
+#define MIN_DMA_RIWT 0x10
/* Tx coalesce parameters */
#define STMMAC_COAL_TX_TIMER 1000
#define STMMAC_MAX_COAL_TX_TICK 100000
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fab65f129207..dc57a2c0a630 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2512,9 +2512,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
priv->tx_lpi_timer = STMMAC_DEFAULT_TWT_LS;
if (priv->use_riwt) {
- ret = stmmac_rx_watchdog(priv, priv->ioaddr, MAX_DMA_RIWT, rx_cnt);
+ ret = stmmac_rx_watchdog(priv, priv->ioaddr, MIN_DMA_RIWT, rx_cnt);
if (!ret)
- priv->rx_riwt = MAX_DMA_RIWT;
+ priv->rx_riwt = MIN_DMA_RIWT;
}
if (priv->hw->pcs)
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 10/10] net: stmmac: Try to get C45 PHY if everything else fails
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
On PCI based setups that are connected to C45 PHY we won't have DT
bindings specifying what's the correct PHY type.
Fallback to C45 if everything else fails when trying to acquire PHY.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bc949665c529..e790ab79e819 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1014,6 +1014,20 @@ static int stmmac_init_phy(struct net_device *dev)
phydev = mdiobus_get_phy(priv->mii, addr);
if (!phydev) {
+ /* Try C45 */
+ phydev = get_phy_device(priv->mii, addr, true);
+ if (phydev && !IS_ERR(phydev)) {
+ ret = phy_device_register(phydev);
+ if (ret) {
+ phy_device_free(phydev);
+ phydev = NULL;
+ }
+ } else {
+ phydev = NULL;
+ }
+ }
+
+ if (!phydev) {
netdev_err(priv->dev, "no phy at addr %d\n", addr);
return -ENODEV;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 08/10] net: stmmac: Update RX Tail Pointer to last free entry
From: Jose Abreu @ 2019-06-26 13:47 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1561556555.git.joabreu@synopsys.com>
Update the RX Tail Pointer to the last available SKB entry.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 65ff2a2a15e4..0893af8b432d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3337,6 +3337,8 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
}
rx_q->dirty_rx = entry;
+ rx_q->rx_tail_addr = rx_q->dma_rx_phy +
+ (rx_q->dirty_rx * sizeof(struct dma_desc));
stmmac_set_rx_tail_ptr(priv, priv->ioaddr, rx_q->rx_tail_addr, queue);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Alex Elder @ 2019-06-26 13:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Johannes Berg, Dan Williams, Subash Abhinov Kasiviswanathan,
abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
DTML, Eric Caruso, evgreen, Ilias Apalodimas, Linux ARM,
linux-arm-msm, Linux Kernel Mailing List, linux-soc, Networking,
syadagir
In-Reply-To: <CAK8P3a1ixL9ZjYz=pWTxvMfeD89S6QxSeHt9ZCL9dkCNV5pMHQ@mail.gmail.com>
On 6/24/19 11:40 AM, Arnd Bergmann wrote:
> On Mon, Jun 24, 2019 at 6:21 PM Alex Elder <elder@linaro.org> wrote:
>> On 6/18/19 2:03 PM, Johannes Berg wrote:
>>
>>> Really there are two possible ways (and they intersect to some extent).
. . .
>>> The other is something like IPA or the Intel modem driver, where the
>>> device is actually a single (e.g. PCIe) device and just has a single
>>> driver, but that single driver offers different channels.
>>
>> What I don't like about this is that it's more monolithic. It
>> seems better to have the low-level IPA or Intel modem driver (or
>> any other driver that can support communication between the AP
>> and WWAN device) present communication paths that other function-
>> specific drivers can attach to and use.
>
> I did not understand Johannes description as two competing models
> for the same code, but rather two kinds of existing hardware that
> a new driver system would have to deal with.
Based on my understanding of what he said in a message I just
responded to, I think you are exactly right.
. . .
> What we should try to avoid though is a way to add driver private
> interfaces that risk having multiple drivers create similar functionality
> in incompatible ways.
Agreed.
-Alex
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Arnd Bergmann @ 2019-06-26 13:58 UTC (permalink / raw)
To: Alex Elder
Cc: Johannes Berg, Dan Williams, Subash Abhinov Kasiviswanathan,
abhishek.esse, Ben Chan, Bjorn Andersson, cpratapa, David Miller,
DTML, Eric Caruso, evgreen, Ilias Apalodimas, Linux ARM,
linux-arm-msm, Linux Kernel Mailing List, linux-soc, Networking,
syadagir
In-Reply-To: <edea19ef-f225-bdcd-f394-77e326d1d3ad@linaro.org>
On Wed, Jun 26, 2019 at 3:39 PM Alex Elder <elder@linaro.org> wrote:
> On 6/25/19 9:19 AM, Johannes Berg wrote:
> > On Mon, 2019-06-24 at 18:40 +0200, Arnd Bergmann wrote:
>
> > So, IOW, I'm not sure I see how you'd split that up. I guess you could
> > if you actually do something like the "rmnet" model, and I suppose
> > you're free to do that for IPA if you like, but I tend to think that's
> > actually a burden, not a win since you just get more complex code that
> > needs to interact with more pieces. A single driver for a single
> > hardware that knows about the few types of channels seems simpler to me.
> >
> >> - to answer Johannes question, my understanding is that the interface
> >> between kernel and firmware/hardware for IPA has a single 'struct
> >> device' that is used for both the data and the control channels,
> >> rather than having a data channel and an independent control device,
> >> so this falls into the same category as the Intel one (please correct
> >> me on that)
>
> I don't think that's quite right, but it might be partially
> right. There is a single device representing IPA, but the
> picture is a little more complicated.
>
> The IPA hardware is actually something that sits *between* the
> AP and the modem. It implements one form of communication
> pathway (IP data), but there are others (including QMI, which
> presents a network-like interface but it's actually implemented
> via clever use of shared memory and interrupts).
Can you clarify how QMI fits in here? Do you mean one has to
talk to both IPA and QMI to use the modem, or are these two
alternative implementations for the same basic purpose?
> > That sounds about the same then, right.
> >
> > Are the control channels to IPA are actually also tunnelled over the
> > rmnet protocol? And even if they are, perhaps they have a different
> > hardware queue or so? That'd be the case for Intel - different hardware
> > queue, same (or at least similar) protocol spoken for the DMA hardware
> > itself, but different contents of the messages obviously.
>
> I want to be careful talking about "control" but for IPA it comes
> from user space. For the purpose of getting initial code upstream,
> all of that control functionality (which was IOCTL based) has been
> removed, and a fixed configuration is assumed.
My previous understanding was that from the hardware perspective
there is only one control interface, which is for IPA. Part of this
is abstracted to user space with ioctl commands to the IPA driver,
and then one must set up rmnet to match these by configuring
an rmnet device over netlink messages from user space, but
rmnet does not have a control protocol with the hardware.
The exception here is the flow control, which is handled using
in-band XON/OFF messages from the modem to rmnet (and
corresponding Acks the other way) that IPA just passes through.
If we also need to talk to QMI, that would be something completely
different though.
Arnd
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Ivan Khoronzhuk @ 2019-06-26 14:01 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David Miller, grygorii.strashko, hawk, brouer, saeedm, leon,
Alexei Starovoitov, linux-kernel, linux-omap, xdp-newbies,
ilias.apalodimas, Network Development, Daniel Borkmann,
jakub.kicinski, John Fastabend
In-Reply-To: <CA+FuTSff=+zqxxmCv3+bNxraigNgx_1Wm5Kn2FM7TTSZV4dnOg@mail.gmail.com>
On Tue, Jun 25, 2019 at 09:36:15PM -0400, Willem de Bruijn wrote:
>On Tue, Jun 25, 2019 at 2:00 PM Ivan Khoronzhuk
><ivan.khoronzhuk@linaro.org> wrote:
>>
>> Add user counter allowing to delete pool only when no users.
>> It doesn't prevent pool from flush, only prevents freeing the
>> pool instance. Helps when no need to delete the pool and now
>> it's user responsibility to free it by calling page_pool_free()
>> while destroying procedure. It also makes to use page_pool_free()
>> explicitly, not fully hidden in xdp unreg, which looks more
>> correct after page pool "create" routine.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>
>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> index f07c518ef8a5..1ec838e9927e 100644
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -101,6 +101,7 @@ struct page_pool {
>> struct ptr_ring ring;
>>
>> atomic_t pages_state_release_cnt;
>> + atomic_t user_cnt;
>
>refcount_t?
yes, thanks.
>
>> };
>>
>> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>> return page->dma_addr;
>> }
>>
>> +/* used to prevent pool from deallocation */
>> +static inline void page_pool_get(struct page_pool *pool)
>> +{
>> + atomic_inc(&pool->user_cnt);
>> +}
>> +
>> static inline bool is_page_pool_compiled_in(void)
>> {
>> #ifdef CONFIG_PAGE_POOL
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index b366f59885c1..169b0e3c870e 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
>> return -ENOMEM;
>>
>> atomic_set(&pool->pages_state_release_cnt, 0);
>> + atomic_set(&pool->user_cnt, 0);
>>
>> if (pool->p.flags & PP_FLAG_DMA_MAP)
>> get_device(pool->p.dev);
>> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
>> kfree(pool);
>> return ERR_PTR(err);
>> }
>> +
>> + page_pool_get(pool);
>> return pool;
>> }
>> EXPORT_SYMBOL(page_pool_create);
>> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>>
>> void __page_pool_free(struct page_pool *pool)
>> {
>> + /* free only if no users */
>> + if (!atomic_dec_and_test(&pool->user_cnt))
>> + return;
>> +
>> WARN(pool->alloc.count, "API usage violation");
>> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 829377cc83db..04bdcd784d2e 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>>
>> mutex_unlock(&mem_id_lock);
>>
>> + if (type == MEM_TYPE_PAGE_POOL)
>> + page_pool_get(xdp_alloc->page_pool);
>> +
>
>need an analogous page_pool_put in xdp_rxq_info_unreg_mem_model? mlx5
>does not use that inverse function, but intel drivers do.
no need, it's put after call to page_pool_free() in unreg workqueue.
>
>> trace_mem_connect(xdp_alloc, xdp_rxq);
>> return 0;
>> err:
>> --
>> 2.17.1
>>
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next] can: dev: call netif_carrier_off() in register_candev()
From: Willem de Bruijn @ 2019-06-26 14:17 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Rasmus Villemoes, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ff8160d4-3357-9b4f-1840-bbe46195da5a@prevas.dk>
On Wed, Jun 26, 2019 at 5:31 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 24/06/2019 19.26, Willem de Bruijn wrote:
> > On Mon, Jun 24, 2019 at 4:34 AM Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> wrote:
> >>
> >> CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev
> >> trigger as suggested, there's a small inconsistency with the link
> >> property: The LED is on initially, stays on when the device is brought
> >> up, and then turns off (as expected) when the device is brought down.
> >>
> >> Make sure the LED always reflects the state of the CAN device.
> >>
> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> >
> > Should this target net?
>
> No, I think this should go through the CAN tree. Perhaps I've
> misunderstood when to use the net-next prefix - is that only for things
> that should be applied directly to the net-next tree? If so, sorry.
I don't see consistent behavior on the list, so this is probably fine.
It would probably help to target can (for fixes) or can-next (for new
features).
Let me reframe the question: should this target can, instead of can-next?
> > Regardless of CONFIG_CAN_LEDS deprecation,
> > this is already not initialized properly if that CONFIG is disabled
> > and a can_led_event call at device probe is a noop.
>
> I'm not sure I understand this part. The CONFIG_CAN_LEDS support for
> showing the state of the interface is implemented via hooking into the
> ndo_open/ndo_stop callbacks, and does not look at or touch the
> __LINK_STATE_NOCARRIER bit at all.
>
> Other than via the netdev LED trigger I don't think one can even observe
> the slightly odd initial state of the __LINK_STATE_NOCARRIER bit for CAN
> devices,
it's still incorrect, though I guess that's moot in practice.
> which is why I framed this as a fix purely to allow the netdev
> trigger to be a closer drop-in replacement for CONFIG_CAN_LEDS.
So the entire CONFIG_CAN_LEDS code is to be removed? What exactly is
this netdev trigger replacement, if not can_led_event? Sorry, I
probably miss some context.
^ permalink raw reply
* Re: samples/bpf compilation failures - 5.2.0
From: Joel Fernandes @ 2019-06-26 14:21 UTC (permalink / raw)
To: Srinivas Ramana
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, open list:BPF (Safe dynamic programs and tools),
bpf, Joel Fernandes (Google)
In-Reply-To: <0bcdd38c-5cdb-0510-573a-9a6098ab2105@codeaurora.org>
On Wed, Jun 26, 2019 at 1:41 AM Srinivas Ramana <sramana@codeaurora.org> wrote:
>
> + Joel if he has seen this issue.
>
I have not seen this issue and it has been some time since I built BPF
samples, sorry. I am mostly building BPF programs through bcc/bpftrace
and Android's build system.
We ought to make samples easier to build though, I remember when I
built it last year there were header related issues which should be
fixed. It could be that new BPF features broke it again.
J.
> On 5/28/2019 2:27 PM, Srinivas Ramana wrote:
> > Hello,
> >
> > I am trying to build samples/bpf in kernel(5.2.0-rc1) but unsuccessful
> > with below errors. Can you help to point what i am missing or if there
> > is some known issue?
> >
> > ==============================8<===================================
> > $ make samples/bpf/
> > LLC=/local/mnt/workspace/tools/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/llc
> > CLANG=/local/mnt/workspace/tools/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > V=1
> > make -C /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel -f
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/Makefile
> > samples/bpf/
> > ................
> > ................
> > ................
> > make KBUILD_MODULES=1 -f ./scripts/Makefile.build obj=samples/bpf
> > (cat /dev/null; ) > samples/bpf/modules.order
> > make -C
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../tools/lib/bpf/
> > RM='rm -rf' LDFLAGS=
> > srctree=/local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../
> > O=
> >
> > Auto-detecting system features:
> > ... libelf: [ on ]
> > ... bpf: [ on ]
> >
> > make -C
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../..//tools/build
> > CFLAGS= LDFLAGS= fixdep
> > make -f
> > /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../..//tools/build/Makefile.build
> > dir=. obj=fixdep
> > ld -r -o fixdep-in.o fixdep.o
> > ld: fixdep.o: Relocations in generic ELF (EM: 183)
> > ld: fixdep.o: Relocations in generic ELF (EM: 183)
> > fixdep.o: error adding symbols: File in wrong format
> > make[5]: *** [fixdep-in.o] Error 1
> > make[4]: *** [fixdep-in.o] Error 2
> > make[3]: *** [fixdep] Error 2
> > make[2]: ***
> > [/local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../tools/lib/bpf/libbpf.a]
> > Error 2
> > make[1]: *** [samples/bpf/] Error 2
> > make: *** [sub-make] Error 2
> > ==============================>8=======================================
> >
> >
> > I am using the below commands to build:
> > ========================================================
> > export ARCH=arm64
> > export CROSS_COMPILE=<path>linaro-toolchain/5.1/bin/aarch64-linux-gnu-
> > export CLANG_TRIPLE=arm64-linux-gnu-
> >
> > make
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > defconfig
> >
> > make
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > -j8
> >
> > make
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > headers_install INSTALL_HDR_PATH=./usr
> >
> > make samples/bpf/
> > LLC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/llc
> > CLANG=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> > V=1
> > CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> >
> > ========================================================
> >
> > Thanks,
> > -- Srinivas R
> >
>
>
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation
> Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
> Collaborative Project
^ permalink raw reply
* Re: [PATCH v2 bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Daniel Borkmann @ 2019-06-26 14:25 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, ast, sdf, bpf, netdev,
kernel-team
In-Reply-To: <20190621045555.4152743-4-andriin@fb.com>
On 06/21/2019 06:55 AM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 207 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 8 ++
> tools/lib/bpf/libbpf.map | 2 +
> 3 files changed, 217 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2bb1fa008be3..d506772df350 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3969,6 +3969,213 @@ int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
> return 0;
> }
>
> +static int parse_uint(const char *buf)
> +{
> + int ret;
> +
> + errno = 0;
> + ret = (int)strtol(buf, NULL, 10);
> + if (errno) {
> + ret = -errno;
> + pr_debug("failed to parse '%s' as unsigned int\n", buf);
> + return ret;
> + }
> + if (ret < 0) {
> + pr_debug("failed to parse '%s' as unsigned int\n", buf);
> + return -EINVAL;
> + }
> + return ret;
> +}
> +
> +static int parse_uint_from_file(const char* file)
> +{
> + char buf[STRERR_BUFSIZE];
> + int fd, ret;
> +
> + fd = open(file, O_RDONLY);
> + if (fd < 0) {
> + ret = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + ret = read(fd, buf, sizeof(buf));
> + ret = ret < 0 ? -errno : ret;
> + close(fd);
> + if (ret < 0) {
> + pr_debug("failed to read '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + if (ret == 0 || ret >= sizeof(buf)) {
> + buf[sizeof(buf) - 1] = 0;
> + pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> + return -EINVAL;
> + }
> + return parse_uint(buf);
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> + return parse_uint_from_file(file);
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> + return parse_uint_from_file(file);
> +}
> +
> +static int parse_config_from_file(const char *file)
> +{
> + char buf[STRERR_BUFSIZE];
> + int fd, ret;
> +
> + fd = open(file, O_RDONLY);
> + if (fd < 0) {
> + ret = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + ret = read(fd, buf, sizeof(buf));
> + ret = ret < 0 ? -errno : ret;
> + close(fd);
> + if (ret < 0) {
> + pr_debug("failed to read '%s': %s\n", file,
> + libbpf_strerror_r(ret, buf, sizeof(buf)));
> + return ret;
> + }
> + if (ret == 0 || ret >= sizeof(buf)) {
> + buf[sizeof(buf) - 1] = 0;
> + pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> + return -EINVAL;
> + }
> + if (strncmp(buf, "config:", 7)) {
> + pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> + return -EINVAL;
> + }
> + return parse_uint(buf + 7);
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> + return parse_config_from_file(file);
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> + return parse_config_from_file(file);
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char* name,
> + uint64_t offset, int pid)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int type, pfd, err;
> +
> + type = uprobe ? determine_uprobe_perf_type()
> + : determine_kprobe_perf_type();
> + if (type < 0) {
> + pr_warning("failed to determine %s perf type: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> + return type;
> + }
> + if (retprobe) {
> + int bit = uprobe ? determine_uprobe_retprobe_bit()
> + : determine_kprobe_retprobe_bit();
> +
> + if (bit < 0) {
> + pr_warning("failed to determine %s retprobe bit: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(bit, errmsg,
> + sizeof(errmsg)));
> + return bit;
> + }
> + attr.config |= 1 << bit;
> + }
> + attr.size = sizeof(attr);
> + attr.type = type;
> + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> + attr.config2 = offset; /* kprobe_addr or probe_offset */
> +
> + /* pid filter is meaningful only for uprobes */
> + pfd = syscall(__NR_perf_event_open, &attr,
> + pid < 0 ? -1 : pid /* pid */,
> + pid == -1 ? 0 : -1 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("%s perf_event_open() failed: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +int bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> + const char *func_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> + 0 /* offset */, -1 /* pid */);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return pfd;
> + }
> + err = bpf_program__attach_perf_event(prog, pfd);
> + if (err) {
> + libbpf_perf_event_disable_and_close(pfd);
> + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
I do like that we facilitate usage by adding these APIs to libbpf, but my $0.02
would be that they should be designed slightly different. See it as a nit, but
given it's exposed in libbpf.map and therefore immutable in future it's worth
considering; right now with this set here you have:
int bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
const char *func_name)
int bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
pid_t pid, const char *binary_path,
size_t func_offset)
int bpf_program__attach_tracepoint(struct bpf_program *prog,
const char *tp_category,
const char *tp_name)
int bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
const char *tp_name)
int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
int libbpf_perf_event_disable_and_close(int pfd)
So the idea is that all the bpf_program__attach_*() APIs return an fd that you
can later on pass into libbpf_perf_event_disable_and_close(). I think there is
a bit of a disconnect in that the bpf_program__attach_*() APIs try to do too
many things at once. For example, the bpf_program__attach_raw_tracepoint() fd
has nothing to do with perf, so passing to libbpf_perf_event_disable_and_close()
kind of works, but is hacky since there's no PERF_EVENT_IOC_DISABLE for it so this
would always error if a user cares to check the return code. In the kernel, we
use anon inode for this kind of object. Also, if a user tries to add more than
one program to the same event, we need to recreate a new event fd every time.
What this boils down to is that this should get a proper abstraction, e.g. as
in struct libbpf_event which holds the event object. There should be helper
functions like libbpf_event_create_{kprobe,uprobe,tracepoint,raw_tracepoint} returning
such an struct libbpf_event object on success, and a single libbpf_event_destroy()
that does the event specific teardown. bpf_program__attach_event() can then take
care of only attaching the program to it. Having an object for this is also more
extensible than just a fd number. Nice thing is that this can also be completely
internal to libbpf.c as with struct bpf_program and other abstractions where we
don't expose the internals in the public header.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH] xsk: Properly terminate assignment in xskq_produce_flush_desc
From: Daniel Borkmann @ 2019-06-26 14:30 UTC (permalink / raw)
To: Nathan Chancellor, Björn Töpel, Magnus Karlsson,
David S. Miller, Alexei Starovoitov, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend
Cc: netdev, bpf, xdp-newbies, linux-kernel, clang-built-linux,
Nick Desaulniers, Nathan Huckleberry
In-Reply-To: <20190625182352.13918-1-natechancellor@gmail.com>
On 06/25/2019 08:23 PM, Nathan Chancellor wrote:
> Clang warns:
>
> In file included from net/xdp/xsk_queue.c:10:
> net/xdp/xsk_queue.h:292:2: warning: expression result unused
> [-Wunused-value]
> WRITE_ONCE(q->ring->producer, q->prod_tail);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/compiler.h:284:6: note: expanded from macro 'WRITE_ONCE'
> __u.__val; \
> ~~~ ^~~~~
> 1 warning generated.
>
> The q->prod_tail assignment has a comma at the end, not a semi-colon.
> Fix that so clang no longer warns and everything works as expected.
>
> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> Link: https://github.com/ClangBuiltLinux/linux/issues/544
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH bpf] tools: bpftool: use correct argument in cgroup errors
From: Daniel Borkmann @ 2019-06-26 14:30 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov
Cc: netdev, bpf, oss-drivers, guro, sdf, Quentin Monnet
In-Reply-To: <20190625165631.18928-1-jakub.kicinski@netronome.com>
On 06/25/2019 06:56 PM, Jakub Kicinski wrote:
> cgroup code tries to use argv[0] as the cgroup path,
> but if it fails uses argv[1] to report errors.
>
> Fixes: 5ccda64d38cc ("bpftool: implement cgroup bpf operations")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Applied, thanks!
^ 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