Netdev List
 help / color / mirror / Atom feed
* ·         can you assist me?
From: Laura @ 2019-06-26 11:16 UTC (permalink / raw)
  To: netdev

Hi,

Did you see my email below from last week?

We manufacture ALL custom LOGO and branded products – over 300,000 to
choose from.

The most asked about product that we make, are the custom printed USB flash
drives!
We can print your logo on them and load your digital images, videos and
files!

http://inaflashweb.com

Here is what we include:
-Any size memory you need: 64MB up to 128GB
-We will print your logo on both sides, just ask!
-Very Low Order Minimums
-Need them quickly? Not a problem, we offer Rush Service
http://inaflashweb.com

NEW: We can make a custom shaped USB drive to look like your Logo or
product!

Email over a copy of your logo and we will create a design mock up for you
at no cost!

Our higher memory sizes are a really good option right now!

Ask about the “Double Your Memory” upgrade promotion going on right
now!

Pricing is low right now, so let us know what you need and we will get you
a quick quote.

We will beat any competitors pricing, send us your last invoice and we will
beat it!

We always offer great rates for schools and nonprofits as well.

Let us know what you would like quoted?
http://inaflashweb.com

Regards,



Sabrina Millons
+1-888-480-8218
Custom USB Account Manager
http://inaflashweb.com


^ permalink raw reply

* Re: [PATCH nf-next v2 2/2] netfilter: nft_meta: Add NFT_META_BRI_VLAN support
From: wenxu @ 2019-06-26 12:42 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <20190626102935.ztxcfb3kysvohzi3@salvia>

I agree with you, It's a more generic way to set the vlan tag not base on

any bridge. I will resubmit NFT_META_BRI_VLAN_PROTO and

NFT_META_VLAN patches

在 2019/6/26 18:29, Pablo Neira Ayuso 写道:
> Could you add a NFT_META_BRI_VLAN_PROTO? Similar to patch 1/2, to
> retrieve p->br->vlan_proto.
>
> Then, add a generic way to set the vlan metadata. I'm attaching an
> incomplete patch, so there is something like:
>
>         meta vlan set 0x88a8:20
>
> to set q-in-q.
>
> we could also add a shortcut for simple vlan case (no q-in-q), ie.
> assuming protocol is 0x8100:
>
>         meta vlan set 20
>
> Does this make sense to you?
>
> And we have a way to set the meta vlan information from ingress to
> then, which is something I also need here.
>
> Thanks.

^ permalink raw reply

* RE: [PATCH net-next 0/2] net: ipv4: remove erroneous advancement of list pointer
From: Ran Rozenstein @ 2019-06-26 12:48 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Tariq Toukan, netdev@vger.kernel.org, Maor Gottlieb,
	edumazet@google.com
In-Reply-To: <20190625091903.gepfjgpiksslnyqy@breakpoint.cc>



> -----Original Message-----
> From: Florian Westphal [mailto:fw@strlen.de]
> Sent: Tuesday, June 25, 2019 12:19
> To: Ran Rozenstein <ranro@mellanox.com>
> Cc: Tariq Toukan <tariqt@mellanox.com>; Florian Westphal <fw@strlen.de>;
> netdev@vger.kernel.org; Maor Gottlieb <maorg@mellanox.com>;
> edumazet@google.com
> Subject: Re: [PATCH net-next 0/2] net: ipv4: remove erroneous
> advancement of list pointer
> 
> Ran Rozenstein <ranro@mellanox.com> wrote:
> > > On 6/17/2019 5:02 PM, Florian Westphal wrote:
> > > > Tariq reported a soft lockup on net-next that Mellanox was able to
> > > > bisect to 2638eb8b50cf ("net: ipv4: provide __rcu annotation for
> ifa_list").
> > > >
> > > > While reviewing above patch I found a regression when addresses
> > > > have a lifetime specified.
> > > >
> > > > Second patch extends rtnetlink.sh to trigger crash (without first
> > > > patch applied).
> > > >
> > >
> > > Thanks Florian.
> > >
> > > Ran, can you please test?
> >
> > Tested, still reproduce.
> 
> Can you be a little more specific? Is there any reproducer?

The test dose stress on the interface by running this 2 commands in loop:

command is: /sbin/ip -f inet addr add $IP/16 brd + dev ens8f1
command is: ifconfig ens8f1 $IP netmask 255.255.0.0

when $IP change every iteration.

It execute every second when we see the reproduce somewhere between 40 to 200 seconds of execution.

Thanks,
Ran




^ permalink raw reply

* Re: net: never suspend the ethernet PHY on certain boards?
From: Andrew Lunn @ 2019-06-26 12:55 UTC (permalink / raw)
  To: Alexander Dahl; +Cc: netdev, linux-kernel, Thomas Pfahl
In-Reply-To: <4693980.Yko7hG0E1C@ada>

> What I could do:
> 
> 1) Revert that change on my tree, which would mean reverting a generic bugfix
> 2) Patch smsc phy driver to not suspend anymore
> 3) Invent some new way to prevent suspend on a configuration basis (dt?)
> 4) Anything I did not think of yet

5) Enable WoL?

There are other boards which have PHY clock issues. Let me check how
they work around this.

     Andrew

^ permalink raw reply

* Re: [PATCH nf-next v2 2/2] netfilter: nft_meta: Add NFT_META_BRI_VLAN support
From: Pablo Neira Ayuso @ 2019-06-26 12:57 UTC (permalink / raw)
  To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <b037a0a9-4729-41ff-81bb-ca76c0e3fba9@ucloud.cn>

On Wed, Jun 26, 2019 at 08:42:27PM +0800, wenxu wrote:
> I agree with you, It's a more generic way to set the vlan tag not base on
> 
> any bridge. I will resubmit NFT_META_BRI_VLAN_PROTO and
> 
> NFT_META_VLAN patches

Thank you very much.

^ permalink raw reply

* [PATCH resend] can: rcar_canfd: fix possible IRQ storm on high load
From: Nikita Yushchenko @ 2019-06-26 13:08 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Simon Horman, Wolfram Sang
  Cc: linux-can, netdev, linux-kernel, Artemi Ivanov, Sergei Shtylyov,
	Nikita Yushchenko

We have observed rcar_canfd driver entering IRQ storm under high load,
with following scenario:
- rcar_canfd_global_interrupt() in entered due to Rx available,
- napi_schedule_prep() is called, and sets NAPIF_STATE_SCHED in state
- Rx fifo interrupts are masked,
- rcar_canfd_global_interrupt() is entered again, this time due to
  error interrupt (e.g. due to overflow),
- since scheduled napi poller has not yet executed, condition for calling
  napi_schedule_prep() from rcar_canfd_global_interrupt() remains true,
  thus napi_schedule_prep() gets called and sets NAPIF_STATE_MISSED flag
  in state,
- later, napi poller function rcar_canfd_rx_poll() gets executed, and
  calls napi_complete_done(),
- due to NAPIF_STATE_MISSED flag in state, this call does not clear
  NAPIF_STATE_SCHED flag from state,
- on return from napi_complete_done(), rcar_canfd_rx_poll() unmasks Rx
  interrutps,
- Rx interrupt happens, rcar_canfd_global_interrupt() gets called
  and calls napi_schedule_prep(),
- since NAPIF_STATE_SCHED is set in state at this time, this call
  returns false,
- due to that false return, rcar_canfd_global_interrupt() returns
  without masking Rx interrupt
- and this results into IRQ storm: unmasked Rx interrupt happens again
  and again is misprocessed in the same way.

This patch fixes that scenario by unmasking Rx interrupts only when
napi_complete_done() returns true, which means it has cleared
NAPIF_STATE_SCHED in state.

Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
 drivers/net/can/rcar/rcar_canfd.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 05410008aa6b..de34a4b82d4a 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1508,10 +1508,11 @@ static int rcar_canfd_rx_poll(struct napi_struct *napi, int quota)
 
 	/* All packets processed */
 	if (num_pkts < quota) {
-		napi_complete_done(napi, num_pkts);
-		/* Enable Rx FIFO interrupts */
-		rcar_canfd_set_bit(priv->base, RCANFD_RFCC(ridx),
-				   RCANFD_RFCC_RFIE);
+		if (napi_complete_done(napi, num_pkts)) {
+			/* Enable Rx FIFO interrupts */
+			rcar_canfd_set_bit(priv->base, RCANFD_RFCC(ridx),
+					   RCANFD_RFCC_RFIE);
+		}
 	}
 	return num_pkts;
 }
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH resend] can: rcar_canfd: fix possible IRQ storm on high load
From: Wolfram Sang @ 2019-06-26 13:12 UTC (permalink / raw)
  To: Nikita Yushchenko, Ramesh Shanmugasundaram
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Simon Horman, Wolfram Sang, linux-can, netdev, linux-kernel,
	Artemi Ivanov, Sergei Shtylyov
In-Reply-To: <20190626130848.6671-1-nikita.yoush@cogentembedded.com>

[-- Attachment #1: Type: text/plain, Size: 2605 bytes --]

On Wed, Jun 26, 2019 at 04:08:48PM +0300, Nikita Yushchenko wrote:
> We have observed rcar_canfd driver entering IRQ storm under high load,
> with following scenario:
> - rcar_canfd_global_interrupt() in entered due to Rx available,
> - napi_schedule_prep() is called, and sets NAPIF_STATE_SCHED in state
> - Rx fifo interrupts are masked,
> - rcar_canfd_global_interrupt() is entered again, this time due to
>   error interrupt (e.g. due to overflow),
> - since scheduled napi poller has not yet executed, condition for calling
>   napi_schedule_prep() from rcar_canfd_global_interrupt() remains true,
>   thus napi_schedule_prep() gets called and sets NAPIF_STATE_MISSED flag
>   in state,
> - later, napi poller function rcar_canfd_rx_poll() gets executed, and
>   calls napi_complete_done(),
> - due to NAPIF_STATE_MISSED flag in state, this call does not clear
>   NAPIF_STATE_SCHED flag from state,
> - on return from napi_complete_done(), rcar_canfd_rx_poll() unmasks Rx
>   interrutps,
> - Rx interrupt happens, rcar_canfd_global_interrupt() gets called
>   and calls napi_schedule_prep(),
> - since NAPIF_STATE_SCHED is set in state at this time, this call
>   returns false,
> - due to that false return, rcar_canfd_global_interrupt() returns
>   without masking Rx interrupt
> - and this results into IRQ storm: unmasked Rx interrupt happens again
>   and again is misprocessed in the same way.
> 
> This patch fixes that scenario by unmasking Rx interrupts only when
> napi_complete_done() returns true, which means it has cleared
> NAPIF_STATE_SCHED in state.
> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>

CCing the driver author...

> ---
>  drivers/net/can/rcar/rcar_canfd.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 05410008aa6b..de34a4b82d4a 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1508,10 +1508,11 @@ static int rcar_canfd_rx_poll(struct napi_struct *napi, int quota)
>  
>  	/* All packets processed */
>  	if (num_pkts < quota) {
> -		napi_complete_done(napi, num_pkts);
> -		/* Enable Rx FIFO interrupts */
> -		rcar_canfd_set_bit(priv->base, RCANFD_RFCC(ridx),
> -				   RCANFD_RFCC_RFIE);
> +		if (napi_complete_done(napi, num_pkts)) {
> +			/* Enable Rx FIFO interrupts */
> +			rcar_canfd_set_bit(priv->base, RCANFD_RFCC(ridx),
> +					   RCANFD_RFCC_RFIE);
> +		}
>  	}
>  	return num_pkts;
>  }
> -- 
> 2.11.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 04/12] net: sched: add tcf_block_setup()
From: Pablo Neira Ayuso @ 2019-06-26 13:16 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, netfilter-devel, davem, thomas.lendacky, f.fainelli,
	ariel.elior, michael.chan, santosh, madalin.bucur, yisen.zhuang,
	salil.mehta, jeffrey.t.kirsher, tariqt, saeedm, jiri, idosch,
	jakub.kicinski, peppe.cavallaro, grygorii.strashko, andrew,
	vivien.didelot, alexandre.torgue, joabreu, linux-net-drivers,
	ganeshgr, ogerlitz, Manish.Chopra, marcelo.leitner, mkubecek,
	venkatkumar.duvvuru, cphealy
In-Reply-To: <20190626121256.GA2424@nanopsycho>

On Wed, Jun 26, 2019 at 02:12:56PM +0200, Jiri Pirko wrote:
> Tue, Jun 25, 2019 at 10:31:54AM CEST, pablo@netfilter.org wrote:
> >On Fri, Jun 21, 2019 at 07:16:03PM +0200, Jiri Pirko wrote:
> >> Thu, Jun 20, 2019 at 09:49:09PM CEST, pablo@netfilter.org wrote:
> >> 
> >> [...]
> >> 
> >> > 
> >> >+static LIST_HEAD(tcf_block_cb_list);
> >> 
> >> I still don't like the global list. Have to go throught the code more
> >> carefully, but why you can't pass the priv/ctx from tc/netfilter. From
> >> tc it would be tcf_block as it is now, from netfilter something else.
> >
> >This tcf_block_cb_list should go away at some point, once drivers know
> >how to deal with multiple subsystems using the setup block
> >infrastructure. As I said in my previous email, only one can set up
> >the block at this stage, the ones coming later will hit busy.
> 
> The driver should know if it can bind or is busy. Also, the bind cmd
> should contain type of binder (tc/nft/whatever) or perhaps rather binder
> priority (according to the hook order in rx/tx).

OK, so I see two possible paths then:

#1 Add global list and allow one single subsystem to bind by now. Then
   later, in a follow up patchset. Add binder type and priority once
   there is a driver that can handle the three subsystems, remove
   this global list and each driver deals/knows what to do from the
   binder path.

#2 Remove the global list now, each driver maintains a list of flow blocks
   internally, allow one single flow block by now. This will need a bit more
   code, since there will be code in the driver to maintain the list of
   existing flow blocks, per driver, instead of global. So it will be
   a per-driver global local to check if there is a flow block with
   this [ cb, cb_ident ] already in place.

#1 is almost ready - it's this batch :-) -  then #2 may need more code -
this batch is slightly large.

I understand though that path #2 may make it easier for the first
driver client allowing for the three subsystems to bind.

Let me know what path your prefer.

Thanks for reviewing.

^ permalink raw reply

* RE: [PATCH net-next 01/16] qlge: Remove irq_cnt
From: Manish Chopra @ 2019-06-26 13:21 UTC (permalink / raw)
  To: Benjamin Poirier; +Cc: GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <20190626113619.GA27420@f1>

> In msix mode there's no need to explicitly disable completion interrupts, they
> are reliably auto-masked, according to my observations.
> I tested this on two QLE8142 adapters.
> 
> Do you have reason to believe this might not always be the case?

How did you check auto-masking of MSI-X interrupts ?
I was just wondering about the below comment in ql_disable_completion_interrupt(), where for MSI-X it does disable completion intr for zeroth intr.
Seems special case for zeroth intr in MSI-X particular to this device.

        /* HW disables for us if we're MSIX multi interrupts and
         * it's not the default (zeroeth) interrupt.
         */
        if (likely(test_bit(QL_MSIX_ENABLED, &qdev->flags) && intr))
                return 0;



^ permalink raw reply

* Re: [PATCH] rt2x00: fix rx queue hang
From: Soeren Moch @ 2019-06-26 13:28 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: Helmut Schaa, Kalle Valo, David S. Miller, linux-wireless, netdev,
	linux-kernel, stable
In-Reply-To: <20190625095734.GA2886@redhat.com>

On 25.06.19 11:57, Stanislaw Gruszka wrote:
> Hello
>
> On Fri, Jun 21, 2019 at 01:30:01PM +0200, Soeren Moch wrote:
>> On 18.06.19 11:34, Stanislaw Gruszka wrote:
>>> Hi
>>>
>>> On Mon, Jun 17, 2019 at 11:46:56AM +0200, Soeren Moch wrote:
>>>> Since commit ed194d136769 ("usb: core: remove local_irq_save() around
>>>>  ->complete() handler") the handlers rt2x00usb_interrupt_rxdone() and
>>>> rt2x00usb_interrupt_txdone() are not running with interrupts disabled
>>>> anymore. So these handlers are not guaranteed to run completely before
>>>> workqueue processing starts. So only mark entries ready for workqueue
>>>> processing after proper accounting in the dma done queue.
>>> It was always the case on SMP machines that rt2x00usb_interrupt_{tx/rx}done
>>> can run concurrently with rt2x00_work_{rx,tx}done, so I do not
>>> understand how removing local_irq_save() around complete handler broke
>>> things.
>> I think because completion handlers can be interrupted now and scheduled
>> away
>> in the middle of processing.
>>> Have you reverted commit ed194d136769 and the revert does solve the problem ?
>> Yes, I already sent a patch for this, see [1]. But this was not considered
>> an acceptablesolution. Especially RT folks do not like code running with
>> interrupts disabled,particularly when trying to acquire spinlocks then.
>>
>> [1] https://lkml.org/lkml/2019/5/31/863
>>> Between 4.19 and 4.20 we have some quite big changes in rt2x00 driver:
>>>
>>> 0240564430c0 rt2800: flush and txstatus rework for rt2800mmio
>>> adf26a356f13 rt2x00: use different txstatus timeouts when flushing
>>> 5022efb50f62 rt2x00: do not check for txstatus timeout every time on tasklet
>>> 0b0d556e0ebb rt2800mmio: use txdone/txstatus routines from lib
>>> 5c656c71b1bf rt2800: move usb specific txdone/txstatus routines to rt2800lib
>>>
>>> so I'm a bit afraid that one of those changes is real cause of
>>> the issue not ed194d136769 .
>> I tested 4.20 and 5.1 and see the exact same behavior. Reverting this
>> usb core patchsolves the problem.
>> 4.19.x (before this usb core patch) is running fine.
>>>> Note that rt2x00usb_work_rxdone() processes all available entries, not
>>>> only such for which queue_work() was called.
>>>>
>>>> This fixes a regression on a RT5370 based wifi stick in AP mode, which
>>>> suddenly stopped data transmission after some period of heavy load. Also
>>>> stopping the hanging hostapd resulted in the error message "ieee80211
>>>> phy0: rt2x00queue_flush_queue: Warning - Queue 14 failed to flush".
>>>> Other operation modes are probably affected as well, this just was
>>>> the used testcase.
>>> Do you know what actually make the traffic stop,
>>> TX queue hung or RX queue hung?
>> I think RX queue hang, as stated in the patch title. "Queue 14" means QID_RX
>> (rt2x00queue.h, enum data_queue_qid).
>> I also tried to re-add local_irq_save() in only one of the handlers. Adding
>> this tort2x00usb_interrupt_rxdone() alone solved the issue, while doing so
>> for tx alonedid not.
>>
>> Note that this doesn't mean there is no problem for tx, that's maybe
>> just more
>> difficult to trigger.
>>>> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
>>>> index 1b08b01db27b..9c102a501ee6 100644
>>>> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
>>>> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
>>>> @@ -263,9 +263,9 @@ EXPORT_SYMBOL_GPL(rt2x00lib_dmastart);
>>>>
>>>>  void rt2x00lib_dmadone(struct queue_entry *entry)
>>>>  {
>>>> -	set_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags);
>>>>  	clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
>>>>  	rt2x00queue_index_inc(entry, Q_INDEX_DMA_DONE);
>>>> +	set_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags);
>>> Unfortunately I do not understand how this suppose to fix the problem,
>>> could you elaborate more about this change?
>>>
>> Re-adding local_irq_save() around thisrt2x00lib_dmadone()solved
>> the issue. So I also tried to reverse the order of these calls.
>> It seems totally plausible to me, that the correct sequence is to
>> first clear the device assignment, then to set the status to dma_done,
>> then to trigger the workqueue processing for this entry. When the handler
>> is scheduled away in the middle of this sequence, now there is no
>> strange state where the entry can be processed by the workqueue while
>> not declared dma_done for it.
>> With this changed sequence there is no need anymore to disable interrupts
>> for solving the hang issue.
> Thanks very much for explanations. However I still do not fully
> understand the issue. Q_INDEX_DMA_DONE index is only checked on TX
> processing (on RX we use only Q_INDEX_DONE and Q_INDEX) and
> ENTRY_OWNER_DEVICE_DATA is already cleared before rt2x00lib_dmadone()
> in rt2x00usb_interrupt_rxdone() .
>
> So I'm not sure how changing the order solve the problem. Looks
> for me that the issue is triggered by some rt2x00lib_dmadone()
> call done on error path (not in rt2x00usb_interrupt_rxdone())
> and it race with this check:
>
>         if (test_and_set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
>             test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
>                 return false;
>
> in rt2x00usb_kick_rx_entry() - we return instead of submit urb.
Hi Stanislaw,

the good news is: your patch below also solves the issue for me. But
removing the ENTRY_DATA_STATUS_PENDING check in
rt2x00usb_kick_rx_entry() alone does not help, while removing this check
in rt2x00usb_work_rxdone() alone does the trick.

So the real race seems to be that the flags set in the completion
handler are not yet visible on the cpu core running the workqueue. And
because the worker is not rescheduled when aborted, the entry can just
wait forever.
Do you think this could make sense?
> I'm somewhat reluctant to change the order, because TX processing
> might relay on it (we first mark we wait for TX status and
> then mark entry is no longer owned by hardware).
OK, maybe it's just good luck that changing the order solves the rx
problem. Or can memory barriers associated with the spinlock in
rt2x00lib_dmadone() be responsible for that?
(I'm testing on a armv7 system, Cortex-A9 quadcore.)

While looking at it, why we double-clear ENTRY_OWNER_DEVICE_DATA in
rt2x00usb_interrupt_rxdone() directly and in rt2x00lib_dmadone() again,
while not doing the same for tx? Would it make more sense to possibly
set ENTRY_DATA_IO_FAILED before clearing ENTRY_OWNER_DEVICE_DATA in
rt2x00usb_interrupt_rxdone() as for tx?
>  However on RX
> side ENTRY_DATA_STATUS_PENDING bit make no sense as we do not
> wait for status. We should remove ENTRY_DATA_STATUS_PENDING on
> RX side and perhaps this also will solve issue you observe.
I agree that removing the unnecessary checks is a good idea in any case.
> Could you please check below patch, if it fixes the problem as well?
At least I could not trigger the problem within transferring 10GB of
data. But maybe the probability for triggering this bug is just lower
because ENTRY_OWNER_DEVICE_DATA is cleared some time before
ENTRY_DATA_STATUS_PENDING is set?

Soeren
>
> Stanislaw
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
> index b6c1344..731e633 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c
> @@ -360,8 +360,7 @@ static void rt2x00usb_work_rxdone(struct work_struct *work)
>  	while (!rt2x00queue_empty(rt2x00dev->rx)) {
>  		entry = rt2x00queue_get_entry(rt2x00dev->rx, Q_INDEX_DONE);
>  
> -		if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
> -		    !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
> +		if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
>  			break;
>  
>  		/*
> @@ -413,8 +412,7 @@ static bool rt2x00usb_kick_rx_entry(struct queue_entry *entry, void *data)
>  	struct queue_entry_priv_usb *entry_priv = entry->priv_data;
>  	int status;
>  
> -	if (test_and_set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
> -	    test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
> +	if (test_and_set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
>  		return false;
>  
>  	rt2x00lib_dmastart(entry);



^ permalink raw reply

* Re: [PATCH bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Daniel Borkmann @ 2019-06-26 13:32 UTC (permalink / raw)
  To: Song Liu, netdev, bpf; +Cc: ast, kernel-team, jannh
In-Reply-To: <20190625182303.874270-2-songliubraving@fb.com>

On 06/25/2019 08:23 PM, Song Liu 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().
> 
> Helper function bpf_capable() is added to check whether the task has got
> permission via /dev/bpf.
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>

[ Lets Cc Jann so he has a chance to review as he was the one who suggested
  the idea. ]

> ---
>  Documentation/ioctl/ioctl-number.txt |  1 +
>  include/linux/bpf.h                  | 12 +++++
>  include/linux/sched.h                |  8 ++++
>  include/uapi/linux/bpf.h             |  5 ++
>  kernel/bpf/arraymap.c                |  2 +-
>  kernel/bpf/cgroup.c                  |  2 +-
>  kernel/bpf/core.c                    |  4 +-
>  kernel/bpf/cpumap.c                  |  2 +-
>  kernel/bpf/devmap.c                  |  2 +-
>  kernel/bpf/hashtab.c                 |  4 +-
>  kernel/bpf/lpm_trie.c                |  2 +-
>  kernel/bpf/offload.c                 |  2 +-
>  kernel/bpf/queue_stack_maps.c        |  2 +-
>  kernel/bpf/reuseport_array.c         |  2 +-
>  kernel/bpf/stackmap.c                |  2 +-
>  kernel/bpf/syscall.c                 | 72 +++++++++++++++++++++-------
>  kernel/bpf/verifier.c                |  2 +-
>  kernel/bpf/xskmap.c                  |  2 +-
>  kernel/fork.c                        |  4 ++
>  net/core/filter.c                    |  6 +--
>  20 files changed, 104 insertions(+), 34 deletions(-)
> 
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index c9558146ac58..19998b99d603 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -327,6 +327,7 @@ Code  Seq#(hex)	Include File		Comments
>  0xB4	00-0F	linux/gpio.h		<mailto:linux-gpio@vger.kernel.org>
>  0xB5	00-0F	uapi/linux/rpmsg.h	<mailto:linux-remoteproc@vger.kernel.org>
>  0xB6	all	linux/fpga-dfl.h
> +0xBP	01-02	uapi/linux/bpf.h	<mailto:bpf@vger.kernel.org>
>  0xC0	00-0F	linux/usb/iowarrior.h
>  0xCA	00-0F	uapi/misc/cxl.h
>  0xCA	10-2F	uapi/misc/ocxl.h
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index a62e7889b0b6..dbba7870f6df 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -14,6 +14,10 @@
>  #include <linux/numa.h>
>  #include <linux/wait.h>
>  #include <linux/u64_stats_sync.h>
> +#include <linux/sched.h>
> +#include <linux/capability.h>
> +
> +#include <asm/current.h>
>  
>  struct bpf_verifier_env;
>  struct perf_event;
> @@ -742,6 +746,12 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>  int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>  				     const union bpf_attr *kattr,
>  				     union bpf_attr __user *uattr);
> +
> +static inline bool bpf_capable(int cap)
> +{
> +	return test_bit(TASK_BPF_FLAG_PERMITTED, &current->bpf_flags) ||
> +		capable(cap);
> +}
>  #else /* !CONFIG_BPF_SYSCALL */
>  static inline struct bpf_prog *bpf_prog_get(u32 ufd)
>  {
> @@ -874,6 +884,8 @@ static inline int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>  {
>  	return -ENOTSUPP;
>  }
> +
> +#define bpf_capable(cap) capable((cap))
>  #endif /* CONFIG_BPF_SYSCALL */
>  
>  static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 11837410690f..ddd33d4476c5 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1200,6 +1200,10 @@ struct task_struct {
>  	unsigned long			prev_lowest_stack;
>  #endif
>  
> +#ifdef CONFIG_BPF_SYSCALL
> +	unsigned long			bpf_flags;
> +#endif

There are plenty of bits available here:

        /* --- cacheline 14 boundary (896 bytes) --- */
        unsigned int               in_execve:1;          /*   896:31  4 */
        unsigned int               in_iowait:1;          /*   896:30  4 */
        unsigned int               restore_sigmask:1;    /*   896:29  4 */
        unsigned int               in_user_fault:1;      /*   896:28  4 */
        unsigned int               no_cgroup_migration:1; /*   896:27  4 */
        unsigned int               frozen:1;             /*   896:26  4 */
        unsigned int               use_memdelay:1;       /*   896:25  4 */

        /* XXX 25 bits hole, try to pack */
        /* XXX 4 bytes hole, try to pack */

Given that bpf is pretty much enabled by default everywhere, I don't think we
should waste so much space in task_struct just for this flag (pretty sure that
task_struct is the equivalent of sk_buff that rather needs a diet). Other options
could be to add to atomic_flags which also still has space.

>  	/*
>  	 * New fields for task_struct should be added above here, so that
>  	 * they are included in the randomized portion of task_struct.
> @@ -1772,6 +1776,10 @@ static inline void set_task_cpu(struct task_struct *p, unsigned int cpu)
>  
>  #endif /* CONFIG_SMP */
>  
> +enum task_struct_bpf_flags {
> +	TASK_BPF_FLAG_PERMITTED,
> +};
> +
>  /*
>   * In order to reduce various lock holder preemption latencies provide an
>   * interface to see if a vCPU is currently running or not.
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index b077507efa3f..ec3ae452cfd7 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3541,4 +3541,9 @@ struct bpf_sysctl {
>  				 */
>  };
>  
> +#define BPF_IOCTL	0xBF
> +
> +#define BPF_DEV_IOCTL_GET_PERM	_IO(BPF_IOCTL, 0x01)
> +#define BPF_DEV_IOCTL_PUT_PERM	_IO(BPF_IOCTL, 0x02)
> +
>  #endif /* _UAPI__LINUX_BPF_H__ */
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 1c65ce0098a9..9ae668fa9185 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -73,7 +73,7 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
>  	bool percpu = attr->map_type == BPF_MAP_TYPE_PERCPU_ARRAY;
>  	int ret, numa_node = bpf_map_attr_numa_node(attr);
>  	u32 elem_size, index_mask, max_entries;
> -	bool unpriv = !capable(CAP_SYS_ADMIN);
> +	bool unpriv = !bpf_capable(CAP_SYS_ADMIN);
>  	u64 cost, array_size, mask64;
>  	struct bpf_map_memory mem;
>  	struct bpf_array *array;
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index c225c42e114a..fd9bea70f8f3 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -777,7 +777,7 @@ cgroup_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  	case BPF_FUNC_get_current_cgroup_id:
>  		return &bpf_get_current_cgroup_id_proto;
>  	case BPF_FUNC_trace_printk:
> -		if (capable(CAP_SYS_ADMIN))
> +		if (bpf_capable(CAP_SYS_ADMIN))
>  			return bpf_get_trace_printk_proto();
>  		/* fall through */
>  	default:
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ad3be85f1411..25c1e3c59699 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -646,7 +646,7 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
>  void bpf_prog_kallsyms_add(struct bpf_prog *fp)
>  {
>  	if (!bpf_prog_kallsyms_candidate(fp) ||
> -	    !capable(CAP_SYS_ADMIN))
> +	    !bpf_capable(CAP_SYS_ADMIN))
>  		return;
>  
>  	spin_lock_bh(&bpf_lock);
> @@ -768,7 +768,7 @@ static int bpf_jit_charge_modmem(u32 pages)
>  {
>  	if (atomic_long_add_return(pages, &bpf_jit_current) >
>  	    (bpf_jit_limit >> PAGE_SHIFT)) {
> -		if (!capable(CAP_SYS_ADMIN)) {
> +		if (!bpf_capable(CAP_SYS_ADMIN)) {
>  			atomic_long_sub(pages, &bpf_jit_current);
>  			return -EPERM;
>  		}
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index 8dff08768087..4c6054626b4f 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -83,7 +83,7 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
>  	u64 cost;
>  	int ret;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return ERR_PTR(-EPERM);
>  
>  	/* check sanity of attributes */
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 40e86a7e0ef0..b7c3785be289 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -83,7 +83,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
>  	u64 cost;
>  	int err;
>  
> -	if (!capable(CAP_NET_ADMIN))
> +	if (!bpf_capable(CAP_NET_ADMIN))
>  		return ERR_PTR(-EPERM);
>  
>  	/* check sanity of attributes */
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 22066a62c8c9..461a75c311a4 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -244,13 +244,13 @@ static int htab_map_alloc_check(union bpf_attr *attr)
>  	BUILD_BUG_ON(offsetof(struct htab_elem, fnode.next) !=
>  		     offsetof(struct htab_elem, hash_node.pprev));
>  
> -	if (lru && !capable(CAP_SYS_ADMIN))
> +	if (lru && !bpf_capable(CAP_SYS_ADMIN))
>  		/* LRU implementation is much complicated than other
>  		 * maps.  Hence, limit to CAP_SYS_ADMIN for now.
>  		 */
>  		return -EPERM;
>  
> -	if (zero_seed && !capable(CAP_SYS_ADMIN))
> +	if (zero_seed && !bpf_capable(CAP_SYS_ADMIN))
>  		/* Guard against local DoS, and discourage production use. */
>  		return -EPERM;
>  
> diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
> index 56e6c75d354d..571962022fdf 100644
> --- a/kernel/bpf/lpm_trie.c
> +++ b/kernel/bpf/lpm_trie.c
> @@ -543,7 +543,7 @@ static struct bpf_map *trie_alloc(union bpf_attr *attr)
>  	u64 cost = sizeof(*trie), cost_per_node;
>  	int ret;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return ERR_PTR(-EPERM);
>  
>  	/* check sanity of attributes */
> diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
> index ba635209ae9a..d3e5378c5a15 100644
> --- a/kernel/bpf/offload.c
> +++ b/kernel/bpf/offload.c
> @@ -366,7 +366,7 @@ struct bpf_map *bpf_map_offload_map_alloc(union bpf_attr *attr)
>  	struct bpf_offloaded_map *offmap;
>  	int err;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return ERR_PTR(-EPERM);
>  	if (attr->map_type != BPF_MAP_TYPE_ARRAY &&
>  	    attr->map_type != BPF_MAP_TYPE_HASH)
> diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
> index f697647ceb54..01d848f1a783 100644
> --- a/kernel/bpf/queue_stack_maps.c
> +++ b/kernel/bpf/queue_stack_maps.c
> @@ -45,7 +45,7 @@ static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
>  /* Called from syscall */
>  static int queue_stack_map_alloc_check(union bpf_attr *attr)
>  {
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* check sanity of attributes */
> diff --git a/kernel/bpf/reuseport_array.c b/kernel/bpf/reuseport_array.c
> index 50c083ba978c..840f38a58c7d 100644
> --- a/kernel/bpf/reuseport_array.c
> +++ b/kernel/bpf/reuseport_array.c
> @@ -154,7 +154,7 @@ static struct bpf_map *reuseport_array_alloc(union bpf_attr *attr)
>  	struct bpf_map_memory mem;
>  	u64 array_size;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return ERR_PTR(-EPERM);
>  
>  	array_size = sizeof(*array);
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 052580c33d26..1eab27b0bc17 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -90,7 +90,7 @@ static struct bpf_map *stack_map_alloc(union bpf_attr *attr)
>  	u64 cost, n_buckets;
>  	int err;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return ERR_PTR(-EPERM);
>  
>  	if (attr->map_flags & ~STACK_CREATE_FLAG_MASK)
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 7713cf39795a..d80b04b6a5fa 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -23,6 +23,8 @@
>  #include <linux/timekeeping.h>
>  #include <linux/ctype.h>
>  #include <linux/nospec.h>
> +#include <linux/miscdevice.h>
> +#include <linux/resource.h>
>  
>  #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PROG_ARRAY || \
>  			   (map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
> @@ -1166,7 +1168,7 @@ static int map_freeze(const union bpf_attr *attr)
>  		err = -EBUSY;
>  		goto err_put;
>  	}
> -	if (!capable(CAP_SYS_ADMIN)) {
> +	if (!bpf_capable(CAP_SYS_ADMIN)) {
>  		err = -EPERM;
>  		goto err_put;
>  	}
> @@ -1616,7 +1618,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
>  
>  	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
>  	    (attr->prog_flags & BPF_F_ANY_ALIGNMENT) &&
> -	    !capable(CAP_SYS_ADMIN))
> +	    !bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	/* copy eBPF program license from user space */
> @@ -1629,11 +1631,12 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
>  	is_gpl = license_is_gpl_compatible(license);
>  
>  	if (attr->insn_cnt == 0 ||
> -	    attr->insn_cnt > (capable(CAP_SYS_ADMIN) ? BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
> +	    attr->insn_cnt > (bpf_capable(CAP_SYS_ADMIN) ?
> +			      BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
>  		return -E2BIG;
>  	if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
>  	    type != BPF_PROG_TYPE_CGROUP_SKB &&
> -	    !capable(CAP_SYS_ADMIN))
> +	    !bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	bpf_prog_load_fixup_attach_type(attr);
> @@ -1861,7 +1864,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>  	struct bpf_prog *prog;
>  	int ret;
>  
> -	if (!capable(CAP_NET_ADMIN))
> +	if (!bpf_capable(CAP_NET_ADMIN))
>  		return -EPERM;
>  
>  	if (CHECK_ATTR(BPF_PROG_ATTACH))
> @@ -1951,7 +1954,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
>  {
>  	enum bpf_prog_type ptype;
>  
> -	if (!capable(CAP_NET_ADMIN))
> +	if (!bpf_capable(CAP_NET_ADMIN))
>  		return -EPERM;
>  
>  	if (CHECK_ATTR(BPF_PROG_DETACH))
> @@ -2007,7 +2010,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
>  static int bpf_prog_query(const union bpf_attr *attr,
>  			  union bpf_attr __user *uattr)
>  {
> -	if (!capable(CAP_NET_ADMIN))
> +	if (!bpf_capable(CAP_NET_ADMIN))
>  		return -EPERM;
>  	if (CHECK_ATTR(BPF_PROG_QUERY))
>  		return -EINVAL;
> @@ -2051,7 +2054,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
>  	struct bpf_prog *prog;
>  	int ret = -ENOTSUPP;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  	if (CHECK_ATTR(BPF_PROG_TEST_RUN))
>  		return -EINVAL;
> @@ -2088,7 +2091,7 @@ static int bpf_obj_get_next_id(const union bpf_attr *attr,
>  	if (CHECK_ATTR(BPF_OBJ_GET_NEXT_ID) || next_id >= INT_MAX)
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	next_id++;
> @@ -2114,7 +2117,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
>  	if (CHECK_ATTR(BPF_PROG_GET_FD_BY_ID))
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	spin_lock_bh(&prog_idr_lock);
> @@ -2148,7 +2151,7 @@ static int bpf_map_get_fd_by_id(const union bpf_attr *attr)
>  	    attr->open_flags & ~BPF_OBJ_FLAG_MASK)
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	f_flags = bpf_get_file_flag(attr->open_flags);
> @@ -2323,7 +2326,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
>  	info.run_time_ns = stats.nsecs;
>  	info.run_cnt = stats.cnt;
>  
> -	if (!capable(CAP_SYS_ADMIN)) {
> +	if (!bpf_capable(CAP_SYS_ADMIN)) {
>  		info.jited_prog_len = 0;
>  		info.xlated_prog_len = 0;
>  		info.nr_jited_ksyms = 0;
> @@ -2641,7 +2644,7 @@ static int bpf_btf_load(const union bpf_attr *attr)
>  	if (CHECK_ATTR(BPF_BTF_LOAD))
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	return btf_new_fd(attr);
> @@ -2654,7 +2657,7 @@ static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
>  	if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	return btf_get_fd_by_id(attr->btf_id);
> @@ -2723,7 +2726,7 @@ static int bpf_task_fd_query(const union bpf_attr *attr,
>  	if (CHECK_ATTR(BPF_TASK_FD_QUERY))
>  		return -EINVAL;
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	if (attr->task_fd_query.flags != 0)
> @@ -2791,7 +2794,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
>  	union bpf_attr attr = {};
>  	int err;
>  
> -	if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
> +	if (sysctl_unprivileged_bpf_disabled && !bpf_capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
>  	err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
> @@ -2886,3 +2889,40 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
>  
>  	return err;
>  }
> +
> +static long bpf_dev_ioctl(struct file *filp,
> +			  unsigned int ioctl, unsigned long arg)
> +{
> +	switch (ioctl) {
> +	case BPF_DEV_IOCTL_GET_PERM:
> +		set_bit(TASK_BPF_FLAG_PERMITTED, &current->bpf_flags);
> +		break;
> +	case BPF_DEV_IOCTL_PUT_PERM:
> +		clear_bit(TASK_BPF_FLAG_PERMITTED, &current->bpf_flags);

I think the get/put for uapi is a bit misleading, first thought at least for
me is on get/put_user() when I read the name.

> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static const struct file_operations bpf_chardev_ops = {
> +	.unlocked_ioctl = bpf_dev_ioctl,
> +};
> +
> +static struct miscdevice bpf_dev = {
> +	.minor		= MISC_DYNAMIC_MINOR,
> +	.name		= "bpf",
> +	.fops		= &bpf_chardev_ops,
> +	.mode		= 0440,
> +	.nodename	= "bpf",

Here's what kvm does:

static struct miscdevice kvm_dev = {
        KVM_MINOR,
        "kvm",
        &kvm_chardev_ops,
};

Is there an actual reason that mode is not 0 by default in bpf case? Why
we need to define nodename?

> +};
> +
> +static int __init bpf_dev_init(void)
> +{
> +	if (misc_register(&bpf_dev))
> +		pr_warn("BPF: Failed to create /dev/bpf. Continue without it...\n");
> +
> +	return 0;
> +}
> +device_initcall(bpf_dev_init);
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0e079b2298f8..79dc4d641cf3 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
>  		env->insn_aux_data[i].orig_idx = i;
>  	env->prog = *prog;
>  	env->ops = bpf_verifier_ops[env->prog->type];
> -	is_priv = capable(CAP_SYS_ADMIN);
> +	is_priv = bpf_capable(CAP_SYS_ADMIN);
>  
>  	/* grab the mutex to protect few globals used by verifier */
>  	if (!is_priv)
> diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
> index ef7338cebd18..06063679c27a 100644
> --- a/kernel/bpf/xskmap.c
> +++ b/kernel/bpf/xskmap.c
> @@ -21,7 +21,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
>  	int cpu, err;
>  	u64 cost;
>  
> -	if (!capable(CAP_NET_ADMIN))
> +	if (!bpf_capable(CAP_NET_ADMIN))
>  		return ERR_PTR(-EPERM);
>  
>  	if (attr->max_entries == 0 || attr->key_size != 4 ||
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 75675b9bf6df..18f914d54d92 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -923,6 +923,10 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
>  #ifdef CONFIG_MEMCG
>  	tsk->active_memcg = NULL;
>  #endif
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +	tsk->bpf_flags = 0;
> +#endif
>  	return tsk;
>  
>  free_stack:
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 2014d76e0d2a..01ccf031849c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5875,7 +5875,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
>  		break;
>  	}
>  
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!bpf_capable(CAP_SYS_ADMIN))
>  		return NULL;
>  
>  	switch (func_id) {
> @@ -6438,7 +6438,7 @@ static bool cg_skb_is_valid_access(int off, int size,
>  		return false;
>  	case bpf_ctx_range(struct __sk_buff, data):
>  	case bpf_ctx_range(struct __sk_buff, data_end):
> -		if (!capable(CAP_SYS_ADMIN))
> +		if (!bpf_capable(CAP_SYS_ADMIN))
>  			return false;
>  		break;
>  	}
> @@ -6450,7 +6450,7 @@ static bool cg_skb_is_valid_access(int off, int size,
>  		case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
>  			break;
>  		case bpf_ctx_range(struct __sk_buff, tstamp):
> -			if (!capable(CAP_SYS_ADMIN))
> +			if (!bpf_capable(CAP_SYS_ADMIN))
>  				return false;
>  			break;
>  		default:
> 


^ permalink raw reply

* Re: [PATCH resend] can: rcar_canfd: fix possible IRQ storm on high load
From: Wolfram Sang @ 2019-06-26 13:32 UTC (permalink / raw)
  To: Nikita Yushchenko
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Simon Horman, Wolfram Sang, linux-can, netdev, linux-kernel,
	Artemi Ivanov, Sergei Shtylyov
In-Reply-To: <20190626131251.GB801@ninjato>

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Wed, Jun 26, 2019 at 03:12:51PM +0200, Wolfram Sang wrote:
> On Wed, Jun 26, 2019 at 04:08:48PM +0300, Nikita Yushchenko wrote:
> > We have observed rcar_canfd driver entering IRQ storm under high load,
> > with following scenario:
> > - rcar_canfd_global_interrupt() in entered due to Rx available,
> > - napi_schedule_prep() is called, and sets NAPIF_STATE_SCHED in state
> > - Rx fifo interrupts are masked,
> > - rcar_canfd_global_interrupt() is entered again, this time due to
> >   error interrupt (e.g. due to overflow),
> > - since scheduled napi poller has not yet executed, condition for calling
> >   napi_schedule_prep() from rcar_canfd_global_interrupt() remains true,
> >   thus napi_schedule_prep() gets called and sets NAPIF_STATE_MISSED flag
> >   in state,
> > - later, napi poller function rcar_canfd_rx_poll() gets executed, and
> >   calls napi_complete_done(),
> > - due to NAPIF_STATE_MISSED flag in state, this call does not clear
> >   NAPIF_STATE_SCHED flag from state,
> > - on return from napi_complete_done(), rcar_canfd_rx_poll() unmasks Rx
> >   interrutps,
> > - Rx interrupt happens, rcar_canfd_global_interrupt() gets called
> >   and calls napi_schedule_prep(),
> > - since NAPIF_STATE_SCHED is set in state at this time, this call
> >   returns false,
> > - due to that false return, rcar_canfd_global_interrupt() returns
> >   without masking Rx interrupt
> > - and this results into IRQ storm: unmasked Rx interrupt happens again
> >   and again is misprocessed in the same way.
> > 
> > This patch fixes that scenario by unmasking Rx interrupts only when
> > napi_complete_done() returns true, which means it has cleared
> > NAPIF_STATE_SCHED in state.
> > 
> > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> 
> CCing the driver author...

Bounced :(


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox