* Re: [PATCH 1/2] net: sh_eth: add support for SH7786
From: Thomas Petazzoni @ 2017-12-08 15:40 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: David S. Miller, Niklas Söderlund, Geert Uytterhoeven,
Simon Horman, netdev, linux-renesas-soc
In-Reply-To: <a35c703f-9546-d4b7-25c4-681726bcea92@cogentembedded.com>
Hello,
On Tue, 5 Dec 2017 22:49:10 +0300, Sergei Shtylyov wrote:
> >>>> This commit adds the sh_eth_cpu_data structure that describes the
> >>>> SH7786 variant of the IP.
> >>>
> >>> The manual seems to be unavailable, so I have to trust you. :-)
> >>
> >> Yes, sadly. However, if you tell me what to double check, I'd be happy
> >> to do so.
> >
> > I have the manual now, will check against it...
> > DaveM, I'm retracting my ACK for the time being.
>
> Starting to look into the manual, the current patch is wrong. SH7786 SoC
> was probably the 1st one to use what we thought was R-Car specific register
> layout. Definite NAK on this version.
Thanks for the feedback. How do we proceed from there ? I don't have
access to a lot of datasheets of the different Renesas SoCs, so it's
not easy to figure out which IP variant the SH7786 is using compared to
other Renesas SoCs.
Just out of curiosity, which specific aspect makes you think the
proposed patch is wrong ? Have you noticed a specific register or field
that isn't compatible with SH_ETH_REG_FAST_SH4 layout ?
Note that my patch makes Ethernet work in practice on SH7784, I have
root over NFS working as we speak. This certainly doesn't mean that the
patch is entirely correct, but it definitely means that the
SH_ETH_REG_FAST_SH4 is close enough to what the SH7786 is using :-)
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
From: Quentin Monnet @ 2017-12-08 15:39 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev, linux-kernel, kernel-team, ast, daniel, jakub.kicinski,
kafai, David Ahern
In-Reply-To: <20171208141251.GA9458@castle>
2017-12-08 14:12 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> On Fri, Dec 08, 2017 at 10:34:16AM +0000, Quentin Monnet wrote:
>> 2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <guro@fb.com>
>>> This patch adds basic cgroup bpf operations to bpftool:
>>> cgroup list, attach and detach commands.
>>>
>>> Usage is described in the corresponding man pages,
>>> and examples are provided.
> [...]
>>> +MAP COMMANDS
>>> +=============
>>> +
>>> +| **bpftool** **cgroup list** *CGROUP*
>>> +| **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
>>> +| **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
>>> +| **bpftool** **cgroup help**
>>> +|
>>> +| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
>>
>> Could you please give the different possible values for ATTACH_TYPE and
>> ATTACH_FLAGS, and provide some documentation for the flags?
>
> I intentionally didn't include the list of possible values, as it depends
> on the exact kernel version, and other bpftool docs are carefully avoiding
> specifying such things.
Do they? As far as I can tell the only other bpftool command that uses
flags is the `bpftool map update`, and it does specify the possible
values for UPDATE_FLAGS (and document them) in the man page.
I don't believe compatibility is an issue here, since the program and
its documentation come together (so they should stay in sync) and are
part of the kernel tree (so the tool should be compatible with the
kernel sources it comes with). My concern is that there is no way to
guess from the current description what the values for ATTACH_FLAG or
ATTACH_TYPE can be, without reading the source code of the program—which
is not exactly user-friendly.
>
> It would be nice to have a way to ask the kernel about provided bpf program types,
> attach types, etc; but I'm not sure that hardcoding it in bpftool docs is
> a good idea.
They are coded into the bpftool that comes with the docs anyway :).
Quentin
^ permalink raw reply
* Re: WireGuard Upstreaming Roadmap (November 2017)
From: David Miller @ 2017-12-08 15:38 UTC (permalink / raw)
To: Jason; +Cc: rumpelsepp, netdev, linux-kernel
In-Reply-To: <CAHmME9rhB-w=EoUJ-EiT1cgJKS44Uz=uJdphsud-BEN1zHtB9A@mail.gmail.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Fri, 8 Dec 2017 03:17:40 +0100
> On Thu, Dec 7, 2017 at 11:22 AM, Stefan Tatschner
> <rumpelsepp@sevenbyte.org> wrote:
>> I have a question which is related to the involved crypto. As far as I
>> have understood the protocol and the concept of wireguard
>> What's your opinion on this?
>
> This thread has been picked up on the WireGuard mailing list, not here.
>
> Since this concerns the interworkings of the protocol and cryptography
> as a whole, as opposed to implementation details of Linux, please do
> not send these inquiries to LKML. Additionally, please start new
> threads for new topics in the future, rather than hijacking a roadmap
> thread.
>
> Look for my answer on the other mailing list. I'll CC you too.
Sorry, you cannot force the discussion of a feature which will be submitted
upstream to occur on a private mailing list.
It is _ABSOLUTELY_ appropriate to discss this on netdev since it is the
netdev community which must consider issues like this when looking at
whether to accept WireGuard upstream.
Jason, this action and response was entirely inappropriate, and please
I'd like you to reply properly to questions about your feature here.
Thank you.
^ permalink raw reply
* Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: Neil Horman @ 2017-12-08 15:37 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: David Laight, 'Xin Long', network dev,
linux-sctp@vger.kernel.org, davem@davemloft.net
In-Reply-To: <20171208145630.GE3328@localhost.localdomain>
On Fri, Dec 08, 2017 at 12:56:30PM -0200, Marcelo Ricardo Leitner wrote:
> On Fri, Dec 08, 2017 at 02:06:04PM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 08 December 2017 13:04
> > ...
> > > @@ -264,8 +264,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> > > frag |= SCTP_DATA_SACK_IMM;
> > > }
> > >
> > > - chunk = sctp_make_datafrag_empty(asoc, sinfo, len, frag,
> > > - 0, GFP_KERNEL);
> > > + chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
> > > + GFP_KERNEL);
> >
> > I know that none of the sctp code is very optimised, but that indirect
> > call is going to be horrid.
>
> Yeah.. but there is no way to avoid the double derreference
> considering we only have the asoc pointer in there and we have to
> reach the contents of the data chunk operations struct, and the .si
> part is the same as 'stream' part as it's a constant offset.
>
> Due to the for() in there, we could add a variable to store
> asoc->stream.si outside the for and then we can do only a single deref
> inside it. Xin, can you please try and see if the generated code is
> different?
>
> Other suggestions?
>
Is it worth replacing the si struct with an index/enum value, and indexing an
array of method pointer structs? That would save you at least one dereference.
Alternatively you could preform the dereference in two steps (i.e. declare an si
pointer on the stack and set it equal to asoc->stream.si, then deref
si->make_datafrag at call time. That will at least give the compiler an
opportunity to preload the first pointer.
Neil
> Marcelo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net] sfc: pass valid pointers from efx_enqueue_unwind
From: Jarod Wilson @ 2017-12-08 15:37 UTC (permalink / raw)
To: Bert Kenward, Dave Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <d8bd90b3-913e-bd8d-6c4e-8409ecaf9c1a@solarflare.com>
On 2017-12-07 12:18 PM, Bert Kenward wrote:
> The bytes_compl and pkts_compl pointers passed to efx_dequeue_buffers
> cannot be NULL. Add a paranoid warning to check this condition and fix
> the one case where they were NULL.
>
> efx_enqueue_unwind() is called very rarely, during error handling.
> Without this fix it would fail with a NULL pointer dereference in
> efx_dequeue_buffer, with efx_enqueue_skb in the call stack.
>
> Fixes: e9117e5099ea ("sfc: Firmware-Assisted TSO version 2")
> Reported-by: Jarod Wilson <jarod@redhat.com>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>
I didn't have the warn, but the rest is identical to what I did locally
to get around this when I was hitting it.
Tested-by: Jarod Wilson <jarod@redhat.com>
Acked-by: Jarod Wilson <jarod@redhat.com>
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply
* Re: [PATCH net-next] netdevsim: remove check on return value of debugfs_create_dir
From: David Miller @ 2017-12-08 15:36 UTC (permalink / raw)
To: bhole_prashant_q7; +Cc: netdev, jakub.kicinski
In-Reply-To: <20171208021456.3392-1-bhole_prashant_q7@lab.ntt.co.jp>
From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Date: Fri, 8 Dec 2017 11:14:56 +0900
> Initial discussion started about correct handling of this condition.
> Later it was decided to remove this check altogether to make it
> consistent.
>
> Removal of this check isn't fatal to this driver.
...
> @@ -469,8 +469,6 @@ static int __init nsim_module_init(void)
> int err;
>
> nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
> - if (IS_ERR(nsim_ddir))
> - return PTR_ERR(nsim_ddir);
>
> err = bus_register(&nsim_bus);
Please stop this madness.
You cannot continue if this thing returns NULL.
WHY?
Because later if you pass NULL to debugfs_create_dir() in nsim_init() do
you have any idea what it is going to do?
It's going to put the netdevsim device files into the root!
Please:
1) Handle the errors
2) Make them fatal, if DEBUGFS is enabled this should never fail except
for memory allocation failures and we have bigger problems than
successfully loading the netdevsim driver
3) Fix debugfs_create_dir() to have sane return value semantics so that
people do not have to check both NULL and error pointers
Thank you.
^ permalink raw reply
* Re: [PATCH v2] net: sh_eth: do not advertise Gigabit capabilities when not available
From: Thomas Petazzoni @ 2017-12-08 15:36 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: David S. Miller, Niklas Söderlund, Geert Uytterhoeven,
Simon Horman, netdev, linux-renesas-soc
In-Reply-To: <efb99078-3156-666b-0b7e-6c2d1976c89f@cogentembedded.com>
Hello,
On Tue, 5 Dec 2017 22:02:20 +0300, Sergei Shtylyov wrote:
> > + /* mask with MAC supported features */
> > + if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) {
> > + err = phy_set_max_speed(phydev, SPEED_100);
> > + if (err) {
> > + netdev_err(ndev, "failed to limit PHY to 100 Mbit/s\n");
> > + goto err_phy_disconnect;
>
> Er, why do we need a *goto* here at all? Just call phy_disconnect() here
> and be done with that...
Thanks for the feedback, I've sent a v3 that takes into account this
comment.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v8 0/5] Add the ability to do BPF directed error injection
From: Daniel Borkmann @ 2017-12-08 15:35 UTC (permalink / raw)
To: Josef Bacik, rostedt, mingo, davem, netdev, linux-kernel, ast,
kernel-team, linux-btrfs
In-Reply-To: <1512576737-9417-1-git-send-email-josef@toxicpanda.com>
On 12/06/2017 05:12 PM, Josef Bacik wrote:
> Jon noticed that I had a typo in my _ASM_KPROBE_ERROR_INJECT macro. I went to
> figure out why the compiler didn't catch it and it's because it was not used
> anywhere. I had copied it from the trace blacklist code without understanding
> where it was used as cscope didn't find the original macro I was looking for, so
> I assumed it was some voodoo and left it in place. Turns out cscope failed me
> and I didn't need the macro at all, the trace blacklist thing I was looking at
> was for marking assembly functions as blacklisted and I have no intention of
> marking assembly functions as error injectable at the moment.
>
> v7->v8:
> - removed the _ASM_KPROBE_ERROR_INJECT since it was not needed.
The series doesn't apply cleanly to the bpf-next tree, so one last respin with
a rebase would unfortunately still be required, thanks!
^ permalink raw reply
* [PATCH v3] net: sh_eth: do not advertise Gigabit capabilities when not available
From: Thomas Petazzoni @ 2017-12-08 15:35 UTC (permalink / raw)
To: David S. Miller, Sergei Shtylyov, Niklas Söderlund,
Geert Uytterhoeven, Simon Horman
Cc: netdev, linux-renesas-soc, Thomas Petazzoni
Not all variants of the sh_eth hardware have Gigabit
support. Unfortunately, the current driver doesn't tell the PHY about
the limited MAC capabilities. Due to this, if you have a Gigabit
capable PHY, the PHY will advertise its Gigabit capability and
establish a link at 1Gbit/s, even though the MAC doesn't support it.
In order to avoid this, we use the recently introduced
phy_set_max_speed() to tell the PHY to not advertise speed higher than
100 MBit/s.
Tested on a SH7786 platform, with a Gigabit PHY.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v2:
- Drop goto construction used in the phy_set_max_speed() error
handling, as it is not needed. Suggested by Sergei Shtylyov.
Changes since v1:
- Use phy_set_max_speed(), as suggested by Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com>.
---
drivers/net/ethernet/renesas/sh_eth.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index db72d13cebb9..75323000c364 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -1892,6 +1892,16 @@ static int sh_eth_phy_init(struct net_device *ndev)
return PTR_ERR(phydev);
}
+ /* mask with MAC supported features */
+ if (mdp->cd->register_type != SH_ETH_REG_GIGABIT) {
+ int err = phy_set_max_speed(phydev, SPEED_100);
+ if (err) {
+ netdev_err(ndev, "failed to limit PHY to 100 Mbit/s\n");
+ phy_disconnect(phydev);
+ return err;
+ }
+ }
+
phy_attached_info(phydev);
return 0;
--
2.14.3
^ permalink raw reply related
* RE: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: David Laight @ 2017-12-08 15:32 UTC (permalink / raw)
To: 'Marcelo Ricardo Leitner'
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, davem@davemloft.net
In-Reply-To: <20171208151538.GT13341@localhost.localdomain>
From: 'Marcelo Ricardo Leitner'
> Sent: 08 December 2017 15:16
> On Fri, Dec 08, 2017 at 03:01:31PM +0000, David Laight wrote:
> > From: Marcelo Ricardo Leitner
> > > Sent: 08 December 2017 14:57
> > >
> > > On Fri, Dec 08, 2017 at 02:06:04PM +0000, David Laight wrote:
> > > > From: Xin Long
> > > > > Sent: 08 December 2017 13:04
> > > > ...
> > > > > @@ -264,8 +264,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> > > > > frag |= SCTP_DATA_SACK_IMM;
> > > > > }
> > > > >
> > > > > - chunk = sctp_make_datafrag_empty(asoc, sinfo, len, frag,
> > > > > - 0, GFP_KERNEL);
> > > > > + chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
> > > > > + GFP_KERNEL);
> > > >
> > > > I know that none of the sctp code is very optimised, but that indirect
> > > > call is going to be horrid.
> > >
> > > Yeah.. but there is no way to avoid the double derreference
> > > considering we only have the asoc pointer in there and we have to
> > > reach the contents of the data chunk operations struct, and the .si
> > > part is the same as 'stream' part as it's a constant offset.
> > ...
> >
> > It isn't only the double indirect, the indirect call itself isn't 'fun'.
>
> I meant in this context.
>
> The indirect call is so we don't have to flood the stack with
> if (old data chunk fmt) {
> ...
> } else {
> ...
> }
>
> So instead of this, we now have some key operations identified and
> wrapped up behind this struct, allowing us to abstract whatever data
> chunk format it is.
Nothing wrong with:
#define foo(asoc, ...) \
if (asoc->new_fmt) \
foo_new(asoc, __VA_LIST__); \
else \
foo_old(asoc, __VA_LIST__);
> > I think there are other hot paths where you've replaced a sizeof()
> > with a ?: clause.
> > Caching the result might be much better.
>
> The only new ?: clause I could find this patchset is on patch 12 and
> has nothing to do with sizeof().
>
> The sizeof() results are indeed cached, as you can see in patch 4:
> +static struct sctp_stream_interleave sctp_stream_interleave_0 = {
> + .data_chunk_len = sizeof(struct sctp_data_chunk),
> and the two helpers on it at the begining of the patch.
I was getting two bits mixed up.
But the code that reads data_chunk_len is following an awful lot of pointers.
David
^ permalink raw reply
* Re: [PATCH net-next] netdevsim: correctly check return value of debugfs_create_dir
From: David Miller @ 2017-12-08 15:31 UTC (permalink / raw)
To: bhole_prashant_q7; +Cc: netdev, jakub.kicinski
In-Reply-To: <20171208005250.2972-1-bhole_prashant_q7@lab.ntt.co.jp>
From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Date: Fri, 8 Dec 2017 09:52:50 +0900
> Return value is now checked with IS_ERROR_OR_NULL because
> debugfs_create_dir doesn't return error value. It either returns
> NULL or a valid pointer.
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> drivers/net/netdevsim/netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index eb8c679fca9f..88d8ee2c89da 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -469,7 +469,7 @@ static int __init nsim_module_init(void)
> int err;
>
> nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
> - if (IS_ERR(nsim_ddir))
> + if (IS_ERR_OR_NULL(nsim_ddir))
> return PTR_ERR(nsim_ddir);
debugfs_create_dir() should really be fixed, either it uses error pointers
consistently and therefore always provides a suitable error code to return
or it always uses NULL.
This in-between behavior makes using it as an interface painful because
no clear meaning is given to NULL.
So please do the work necessary to make debugfs_create_dir()'s return
semantics clearer and more useful.
Thank you.
^ permalink raw reply
* Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: 'Marcelo Ricardo Leitner' @ 2017-12-08 15:15 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, davem@davemloft.net
In-Reply-To: <67253734d93744a2b3b05d4a0bbe4a8f@AcuMS.aculab.com>
On Fri, Dec 08, 2017 at 03:01:31PM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 08 December 2017 14:57
> >
> > On Fri, Dec 08, 2017 at 02:06:04PM +0000, David Laight wrote:
> > > From: Xin Long
> > > > Sent: 08 December 2017 13:04
> > > ...
> > > > @@ -264,8 +264,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> > > > frag |= SCTP_DATA_SACK_IMM;
> > > > }
> > > >
> > > > - chunk = sctp_make_datafrag_empty(asoc, sinfo, len, frag,
> > > > - 0, GFP_KERNEL);
> > > > + chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
> > > > + GFP_KERNEL);
> > >
> > > I know that none of the sctp code is very optimised, but that indirect
> > > call is going to be horrid.
> >
> > Yeah.. but there is no way to avoid the double derreference
> > considering we only have the asoc pointer in there and we have to
> > reach the contents of the data chunk operations struct, and the .si
> > part is the same as 'stream' part as it's a constant offset.
> ...
>
> It isn't only the double indirect, the indirect call itself isn't 'fun'.
I meant in this context.
The indirect call is so we don't have to flood the stack with
if (old data chunk fmt) {
...
} else {
...
}
So instead of this, we now have some key operations identified and
wrapped up behind this struct, allowing us to abstract whatever data
chunk format it is.
>
> I think there are other hot paths where you've replaced a sizeof()
> with a ?: clause.
> Caching the result might be much better.
The only new ?: clause I could find this patchset is on patch 12 and
has nothing to do with sizeof().
The sizeof() results are indeed cached, as you can see in patch 4:
+static struct sctp_stream_interleave sctp_stream_interleave_0 = {
+ .data_chunk_len = sizeof(struct sctp_data_chunk),
and the two helpers on it at the begining of the patch.
Marcelo
^ permalink raw reply
* Re: Linux 4.14 - regression: broken tun/tap / bridge network with virtio - bisected
From: Jason Wang @ 2017-12-08 15:11 UTC (permalink / raw)
To: Andreas Hartmann, Michal Kubecek; +Cc: David Miller, netdev, Willem de Bruijn
In-Reply-To: <1705a3cc-4d9f-8fd0-3eed-9f6a145d5055@01019freenet.de>
On 2017年12月08日 21:13, Andreas Hartmann wrote:
> On 12/08/2017 at 01:58 PM Michal Kubecek wrote:
>> On Fri, Dec 08, 2017 at 01:45:38PM +0100, Andreas Hartmann wrote:
>>> On 12/08/2017 at 12:40 PM Michal Kubecek wrote:
>>>> On Fri, Dec 08, 2017 at 11:31:50AM +0100, Andreas Hartmann wrote:
>>>>> When will there be a fix for 4.14? It is clearly a regression. Is
>>>>> it possible / a good idea to just remove the complete patch series
>>>>> "Remove UDP Fragmentation Offload support"?
>>>> I cannot give an exact date but the patch is queued for stable (see
>>>> http://patchwork.ozlabs.org/bundle/davem/stable/?state=* ) so that
>>>> it should land in stable-4.14 in near future (weeks at most).
>>> Which one is it? I couldn't find any patch related to this problem at
>>> first glance.
>> "[net,v2] net: accept UFO datagrams from tuntap and packet" - the
>> subject was mentioned in one of my earlier e-mails (with commit id).
> Oh - I thought this would only work during live migration (which doesn't
> happen here). My error.
>
>
> Thanks,
> regards,
> Andreas
I think you can either wait it to go for stable or test Linus tree which
has already contained the patch.
Thanks
^ permalink raw reply
* Re: [PATCH net] tcp: invalidate rate samples during SACK reneging
From: David Miller @ 2017-12-08 15:07 UTC (permalink / raw)
To: ysseung; +Cc: netdev, ncardwell, ycheng
In-Reply-To: <20171207214134.90015-1-ysseung@google.com>
From: Yousuk Seung <ysseung@google.com>
Date: Thu, 7 Dec 2017 13:41:34 -0800
> Mark tcp_sock during a SACK reneging event and invalidate rate samples
> while marked. Such rate samples may overestimate bw by including packets
> that were SACKed before reneging.
>
> < ack 6001 win 10000 sack 7001:38001
> < ack 7001 win 0 sack 8001:38001 // Reneg detected
>> seq 7001:8001 // RTO, SACK cleared.
> < ack 38001 win 10000
>
> In above example the rate sample taken after the last ack will count
> 7001-38001 as delivered while the actual delivery rate likely could
> be much lower i.e. 7001-8001.
>
> This patch adds a new field tcp_sock.sack_reneg and marks it when we
> declare SACK reneging and entering TCP_CA_Loss, and unmarks it after
> the last rate sample was taken before moving back to TCP_CA_Open. This
> patch also invalidates rate samples taken while tcp_sock.is_sack_reneg
> is set.
>
> Fixes: b9f64820fb22 ("tcp: track data delivery rate for a TCP connection")
> Signed-off-by: Yousuk Seung <ysseung@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Priyaranjan Jha <priyarjha@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH v3 05/33] nds32: Exception handling
From: Al Viro @ 2017-12-08 15:05 UTC (permalink / raw)
To: Greentime Hu
Cc: greentime, linux-kernel, arnd, linux-arch, tglx, jason,
marc.zyngier, robh+dt, netdev, deanbo422, devicetree, dhowells,
will.deacon, daniel.lezcano, linux-serial, geert.uytterhoeven,
linus.walleij, mark.rutland, greg, Vincent Chen
In-Reply-To: <8fad3e13c85cd90bd038cac7ead0e97e4438a0e0.1512723245.git.green.hu@gmail.com>
On Fri, Dec 08, 2017 at 05:11:48PM +0800, Greentime Hu wrote:
> diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
> new file mode 100644
> index 0000000..30a275d
> --- /dev/null
> +++ b/arch/nds32/kernel/traps.c
> @@ -0,0 +1,441 @@
> +/*
> + * Copyright (C) 2005-2017 Andes Technology Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/personality.h>
> +#include <linux/kallsyms.h>
> +#include <linux/hardirq.h>
> +#include <linux/kdebug.h>
> +#include <linux/sched/task_stack.h>
> +
> +#include <asm/proc-fns.h>
> +#include <asm/uaccess.h>
The only include of asm/uaccess.h should be in linux/uaccess.h; everything
else should include linux/uaccess.h. The same goes for other patches in
the series.
^ permalink raw reply
* RE: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: David Laight @ 2017-12-08 15:01 UTC (permalink / raw)
To: 'Marcelo Ricardo Leitner'
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, davem@davemloft.net
In-Reply-To: <20171208145630.GE3328@localhost.localdomain>
From: Marcelo Ricardo Leitner
> Sent: 08 December 2017 14:57
>
> On Fri, Dec 08, 2017 at 02:06:04PM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 08 December 2017 13:04
> > ...
> > > @@ -264,8 +264,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> > > frag |= SCTP_DATA_SACK_IMM;
> > > }
> > >
> > > - chunk = sctp_make_datafrag_empty(asoc, sinfo, len, frag,
> > > - 0, GFP_KERNEL);
> > > + chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
> > > + GFP_KERNEL);
> >
> > I know that none of the sctp code is very optimised, but that indirect
> > call is going to be horrid.
>
> Yeah.. but there is no way to avoid the double derreference
> considering we only have the asoc pointer in there and we have to
> reach the contents of the data chunk operations struct, and the .si
> part is the same as 'stream' part as it's a constant offset.
...
It isn't only the double indirect, the indirect call itself isn't 'fun'.
I think there are other hot paths where you've replaced a sizeof()
with a ?: clause.
Caching the result might be much better.
David
^ permalink raw reply
* Re: [PATCHv2 net-next 04/12] sctp: implement make_datafrag for sctp_stream_interleave
From: Marcelo Ricardo Leitner @ 2017-12-08 14:56 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Neil Horman, davem@davemloft.net
In-Reply-To: <2ca21c61e82a44daa29226eac54a4950@AcuMS.aculab.com>
On Fri, Dec 08, 2017 at 02:06:04PM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 08 December 2017 13:04
> ...
> > @@ -264,8 +264,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> > frag |= SCTP_DATA_SACK_IMM;
> > }
> >
> > - chunk = sctp_make_datafrag_empty(asoc, sinfo, len, frag,
> > - 0, GFP_KERNEL);
> > + chunk = asoc->stream.si->make_datafrag(asoc, sinfo, len, frag,
> > + GFP_KERNEL);
>
> I know that none of the sctp code is very optimised, but that indirect
> call is going to be horrid.
Yeah.. but there is no way to avoid the double derreference
considering we only have the asoc pointer in there and we have to
reach the contents of the data chunk operations struct, and the .si
part is the same as 'stream' part as it's a constant offset.
Due to the for() in there, we could add a variable to store
asoc->stream.si outside the for and then we can do only a single deref
inside it. Xin, can you please try and see if the generated code is
different?
Other suggestions?
Marcelo
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations
From: Roman Gushchin @ 2017-12-08 14:53 UTC (permalink / raw)
To: Philippe Ombredanne
Cc: netdev, Quentin Monnet, LKML, kernel-team, ast, daniel,
jakub.kicinski, kafai, David Ahern
In-Reply-To: <CAOFm3uEFF=7DuxsT-+45q2ZENDfhitUg18V53qtOo9fj7JHOvA@mail.gmail.com>
On Fri, Dec 08, 2017 at 02:56:15PM +0100, Philippe Ombredanne wrote:
> On Fri, Dec 8, 2017 at 11:34 AM, Quentin Monnet
> <quentin.monnet@netronome.com> wrote:
> > 2017-12-07 18:39 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> >> This patch adds basic cgroup bpf operations to bpftool:
> >> cgroup list, attach and detach commands.
>
> [...]
> >> --- /dev/null
> >> +++ b/tools/bpf/bpftool/cgroup.c
> >> @@ -0,0 +1,305 @@
> >> +/*
> >> + * Copyright (C) 2017 Facebook
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * as published by the Free Software Foundation; either version
> >> + * 2 of the License, or (at your option) any later version.
> >> + *
> >> + *
> >> + */
> >> +
>
> Roman,
> Have you considered using the simpler and new SPDX ids instead? e.g.:
>
> // SPDX-License-Identifier: GPL-2.0+
> // Copyright (C) 2017 Facebook
> // Author: Roman Gushchin <guro@fb.com>
>
> This would boost your code/comments ratio nicely IMHO.
Thanks, applied to v3!
^ permalink raw reply
* [PATCH v3 net-next 4/4] bpftool: implement cgroup bpf operations
From: Roman Gushchin @ 2017-12-08 14:52 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171208145236.12635-1-guro@fb.com>
This patch adds basic cgroup bpf operations to bpftool:
cgroup list, attach and detach commands.
Usage is described in the corresponding man pages,
and examples are provided.
Syntax:
$ bpftool cgroup list CGROUP
$ bpftool cgroup attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]
$ bpftool cgroup detach CGROUP ATTACH_TYPE PROG
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
---
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 92 +++++++
tools/bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool.rst | 6 +-
tools/bpf/bpftool/cgroup.c | 300 +++++++++++++++++++++
tools/bpf/bpftool/main.c | 3 +-
tools/bpf/bpftool/main.h | 1 +
7 files changed, 401 insertions(+), 5 deletions(-)
create mode 100644 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
create mode 100644 tools/bpf/bpftool/cgroup.c
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
new file mode 100644
index 000000000000..61ded613aee1
--- /dev/null
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -0,0 +1,92 @@
+================
+bpftool-cgroup
+================
+-------------------------------------------------------------------------------
+tool for inspection and simple manipulation of eBPF progs
+-------------------------------------------------------------------------------
+
+:Manual section: 8
+
+SYNOPSIS
+========
+
+ **bpftool** [*OPTIONS*] **cgroup** *COMMAND*
+
+ *OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
+
+ *COMMANDS* :=
+ { **list** | **attach** | **detach** | **help** }
+
+MAP COMMANDS
+=============
+
+| **bpftool** **cgroup list** *CGROUP*
+| **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
+| **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
+| **bpftool** **cgroup help**
+|
+| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
+
+DESCRIPTION
+===========
+ **bpftool cgroup list** *CGROUP*
+ List all programs attached to the cgroup *CGROUP*.
+
+ Output will start with program ID followed by attach type,
+ attach flags and program name.
+
+ **bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
+ Attach program *PROG* to the cgroup *CGROUP* with attach type
+ *ATTACH_TYPE* and optional *ATTACH_FLAGS*.
+
+ **bpftool cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
+ Detach *PROG* from the cgroup *CGROUP* and attach type
+ *ATTACH_TYPE*.
+
+ **bpftool prog help**
+ Print short help message.
+
+OPTIONS
+=======
+ -h, --help
+ Print short generic help message (similar to **bpftool help**).
+
+ -v, --version
+ Print version number (similar to **bpftool version**).
+
+ -j, --json
+ Generate JSON output. For commands that cannot produce JSON, this
+ option has no effect.
+
+ -p, --pretty
+ Generate human-readable JSON output. Implies **-j**.
+
+ -f, --bpffs
+ Show file names of pinned programs.
+
+EXAMPLES
+========
+|
+| **# mount -t bpf none /sys/fs/bpf/**
+| **# mkdir /sys/fs/cgroup/test.slice**
+| **# bpftool prog load ./device_cgroup.o /sys/fs/bpf/prog**
+| **# bpftool cgroup attach /sys/fs/cgroup/test.slice/ device id 1 allow_multi**
+
+**# bpftool cgroup list /sys/fs/cgroup/test.slice/**
+
+::
+
+ ID AttachType AttachFlags Name
+ 1 device allow_multi bpf_prog1
+
+|
+| **# bpftool cgroup detach /sys/fs/cgroup/test.slice/ device id 1**
+| **# bpftool cgroup list /sys/fs/cgroup/test.slice/**
+
+::
+
+ ID AttachType AttachFlags Name
+
+SEE ALSO
+========
+ **bpftool**\ (8), **bpftool-prog**\ (8), **bpftool-map**\ (8)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 9f51a268eb06..421cabc417e6 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -128,4 +128,4 @@ EXAMPLES
SEE ALSO
========
- **bpftool**\ (8), **bpftool-prog**\ (8)
+ **bpftool**\ (8), **bpftool-prog**\ (8), **bpftool-cgroup**\ (8)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index ffdb20e8280f..81c97c0e9b67 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -155,4 +155,4 @@ EXAMPLES
SEE ALSO
========
- **bpftool**\ (8), **bpftool-map**\ (8)
+ **bpftool**\ (8), **bpftool-map**\ (8), **bpftool-cgroup**\ (8)
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index f547a0c0aa34..6732a5a617e4 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -16,7 +16,7 @@ SYNOPSIS
**bpftool** **version**
- *OBJECT* := { **map** | **program** }
+ *OBJECT* := { **map** | **program** | **cgroup** }
*OPTIONS* := { { **-V** | **--version** } | { **-h** | **--help** }
| { **-j** | **--json** } [{ **-p** | **--pretty** }] }
@@ -28,6 +28,8 @@ SYNOPSIS
*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
| **load** | **help** }
+ *CGROUP-COMMANDS* := { **list** | **attach** | **detach** | **help** }
+
DESCRIPTION
===========
*bpftool* allows for inspection and simple modification of BPF objects
@@ -53,4 +55,4 @@ OPTIONS
SEE ALSO
========
- **bpftool-map**\ (8), **bpftool-prog**\ (8)
+ **bpftool-map**\ (8), **bpftool-prog**\ (8), **bpftool-cgroup**\ (8)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
new file mode 100644
index 000000000000..8116d58699af
--- /dev/null
+++ b/tools/bpf/bpftool/cgroup.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (C) 2017 Facebook
+// Author: Roman Gushchin <guro@fb.com>
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <bpf.h>
+
+#include "main.h"
+
+static const char * const attach_type_strings[] = {
+ [BPF_CGROUP_INET_INGRESS] = "ingress",
+ [BPF_CGROUP_INET_EGRESS] = "egress",
+ [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
+ [BPF_CGROUP_SOCK_OPS] = "sock_ops",
+ [BPF_CGROUP_DEVICE] = "device",
+ [__MAX_BPF_ATTACH_TYPE] = NULL,
+};
+
+static enum bpf_attach_type parse_attach_type(const char *str)
+{
+ enum bpf_attach_type type;
+
+ for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
+ if (attach_type_strings[type] &&
+ is_prefix(str, attach_type_strings[type]))
+ return type;
+ }
+
+ return __MAX_BPF_ATTACH_TYPE;
+}
+
+static int list_bpf_prog(int id, const char *attach_type_str,
+ const char *attach_flags_str)
+{
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ int prog_fd;
+
+ prog_fd = bpf_prog_get_fd_by_id(id);
+ if (prog_fd < 0)
+ return -1;
+
+ if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
+ close(prog_fd);
+ return -1;
+ }
+
+ if (json_output) {
+ jsonw_start_object(json_wtr);
+ jsonw_uint_field(json_wtr, "id", info.id);
+ jsonw_string_field(json_wtr, "attach_type",
+ attach_type_str);
+ jsonw_string_field(json_wtr, "attach_flags",
+ attach_flags_str);
+ jsonw_string_field(json_wtr, "name", info.name);
+ jsonw_end_object(json_wtr);
+ } else {
+ printf("%-8u %-15s %-15s %-15s\n", info.id,
+ attach_type_str,
+ attach_flags_str,
+ info.name);
+ }
+
+ close(prog_fd);
+ return 0;
+}
+
+static int list_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
+{
+ __u32 prog_ids[1024] = {0};
+ char *attach_flags_str;
+ __u32 prog_cnt, iter;
+ __u32 attach_flags;
+ char buf[16];
+ int ret;
+
+ prog_cnt = ARRAY_SIZE(prog_ids);
+ ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
+ &prog_cnt);
+ if (ret)
+ return ret;
+
+ if (prog_cnt == 0)
+ return 0;
+
+ switch (attach_flags) {
+ case BPF_F_ALLOW_MULTI:
+ attach_flags_str = "allow_multi";
+ break;
+ case BPF_F_ALLOW_OVERRIDE:
+ attach_flags_str = "allow_override";
+ break;
+ case 0:
+ attach_flags_str = "";
+ break;
+ default:
+ snprintf(buf, sizeof(buf), "unknown(%x)", attach_flags);
+ attach_flags_str = buf;
+ }
+
+ for (iter = 0; iter < prog_cnt; iter++)
+ list_bpf_prog(prog_ids[iter], attach_type_strings[type],
+ attach_flags_str);
+
+ return 0;
+}
+
+static int do_list(int argc, char **argv)
+{
+ enum bpf_attach_type type;
+ int cgroup_fd;
+ int ret = -1;
+
+ if (argc < 1) {
+ p_err("too few parameters for cgroup list\n");
+ goto exit;
+ } else if (argc > 1) {
+ p_err("too many parameters for cgroup list\n");
+ goto exit;
+ }
+
+ cgroup_fd = open(argv[0], O_RDONLY);
+ if (cgroup_fd < 0) {
+ p_err("can't open cgroup %s\n", argv[1]);
+ goto exit;
+ }
+
+ if (json_output)
+ jsonw_start_array(json_wtr);
+ else
+ printf("%-8s %-15s %-15s %-15s\n", "ID", "AttachType",
+ "AttachFlags", "Name");
+
+ for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
+ /*
+ * Not all attach types may be supported, so it's expected,
+ * that some requests will fail.
+ * If we were able to get the list for at least one
+ * attach type, let's return 0.
+ */
+ if (list_attached_bpf_progs(cgroup_fd, type) == 0)
+ ret = 0;
+ }
+
+ if (json_output)
+ jsonw_end_array(json_wtr);
+
+ close(cgroup_fd);
+exit:
+ return ret;
+}
+
+static int do_attach(int argc, char **argv)
+{
+ int cgroup_fd, prog_fd;
+ enum bpf_attach_type attach_type;
+ int attach_flags = 0;
+ int i;
+ int ret = -1;
+
+ if (argc < 4) {
+ p_err("too few parameters for cgroup attach\n");
+ goto exit;
+ }
+
+ cgroup_fd = open(argv[0], O_RDONLY);
+ if (cgroup_fd < 0) {
+ p_err("can't open cgroup %s\n", argv[1]);
+ goto exit;
+ }
+
+ attach_type = parse_attach_type(argv[1]);
+ if (attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach type\n");
+ goto exit_cgroup;
+ }
+
+ argc -= 2;
+ argv = &argv[2];
+ prog_fd = prog_parse_fd(&argc, &argv);
+ if (prog_fd < 0)
+ goto exit_cgroup;
+
+ for (i = 0; i < argc; i++) {
+ if (strcmp(argv[i], "allow_multi") == 0) {
+ attach_flags |= BPF_F_ALLOW_MULTI;
+ } else if (strcmp(argv[i], "allow_override") == 0) {
+ attach_flags |= BPF_F_ALLOW_OVERRIDE;
+ } else {
+ p_err("unknown option: %s\n", argv[i]);
+ goto exit_cgroup;
+ }
+ }
+
+ if (bpf_prog_attach(prog_fd, cgroup_fd, attach_type, attach_flags)) {
+ p_err("failed to attach program");
+ goto exit_prog;
+ }
+
+ if (json_output)
+ jsonw_null(json_wtr);
+
+ ret = 0;
+
+exit_prog:
+ close(prog_fd);
+exit_cgroup:
+ close(cgroup_fd);
+exit:
+ return ret;
+}
+
+static int do_detach(int argc, char **argv)
+{
+ int prog_fd, cgroup_fd;
+ enum bpf_attach_type attach_type;
+ int ret = -1;
+
+ if (argc < 4) {
+ p_err("too few parameters for cgroup detach\n");
+ goto exit;
+ }
+
+ cgroup_fd = open(argv[0], O_RDONLY);
+ if (cgroup_fd < 0) {
+ p_err("can't open cgroup %s\n", argv[1]);
+ goto exit;
+ }
+
+ attach_type = parse_attach_type(argv[1]);
+ if (attach_type == __MAX_BPF_ATTACH_TYPE) {
+ p_err("invalid attach type");
+ goto exit_cgroup;
+ }
+
+ argc -= 2;
+ argv = &argv[2];
+ prog_fd = prog_parse_fd(&argc, &argv);
+ if (prog_fd < 0)
+ goto exit_cgroup;
+
+ if (bpf_prog_detach2(prog_fd, cgroup_fd, attach_type)) {
+ p_err("failed to detach program");
+ goto exit_prog;
+ }
+
+ if (json_output)
+ jsonw_null(json_wtr);
+
+ ret = 0;
+
+exit_prog:
+ close(prog_fd);
+exit_cgroup:
+ close(cgroup_fd);
+exit:
+ return ret;
+}
+
+static int do_help(int argc, char **argv)
+{
+ if (json_output) {
+ jsonw_null(json_wtr);
+ return 0;
+ }
+
+ fprintf(stderr,
+ "Usage: %s %s list CGROUP\n"
+ " %s %s attach CGROUP TYPE PROG [ATTACH_FLAGS]\n"
+ " %s %s detach CGROUP TYPE PROG\n"
+ " %s %s help\n"
+ "\n"
+ " ATTACH_FLAGS := { allow_multi | allow_override }"
+ " " HELP_SPEC_PROGRAM "\n"
+ " " HELP_SPEC_OPTIONS "\n"
+ "",
+ bin_name, argv[-2], bin_name, argv[-2],
+ bin_name, argv[-2], bin_name, argv[-2]);
+
+ return 0;
+}
+
+static const struct cmd cmds[] = {
+ { "list", do_list },
+ { "attach", do_attach },
+ { "detach", do_detach },
+ { "help", do_help },
+ { 0 }
+};
+
+int do_cgroup(int argc, char **argv)
+{
+ return cmd_select(cmds, argc, argv, do_help);
+}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index d294bc8168be..ecd53ccf1239 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -85,7 +85,7 @@ static int do_help(int argc, char **argv)
" %s batch file FILE\n"
" %s version\n"
"\n"
- " OBJECT := { prog | map }\n"
+ " OBJECT := { prog | map | cgroup }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, bin_name, bin_name);
@@ -173,6 +173,7 @@ static const struct cmd cmds[] = {
{ "batch", do_batch },
{ "prog", do_prog },
{ "map", do_map },
+ { "cgroup", do_cgroup },
{ "version", do_version },
{ 0 }
};
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index bec1ccbb49c7..8f6d3cac0347 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -115,6 +115,7 @@ int do_pin_fd(int fd, const char *name);
int do_prog(int argc, char **arg);
int do_map(int argc, char **arg);
+int do_cgroup(int argc, char **arg);
int prog_parse_fd(int *argc, char ***argv);
--
2.14.3
^ permalink raw reply related
* [PATCH v3 net-next 1/4] libbpf: add ability to guess program type based on section name
From: Roman Gushchin @ 2017-12-08 14:52 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171208145236.12635-1-guro@fb.com>
The bpf_prog_load() function will guess program type if it's not
specified explicitly. This functionality will be used to implement
loading of different programs without asking a user to specify
the program type. In first order it will be used by bpftool.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/lib/bpf/libbpf.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5aa45f89da93..205b7822fa0a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1721,6 +1721,45 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
+#define BPF_PROG_SEC(string, type) { string, sizeof(string), type }
+static const struct {
+ const char *sec;
+ size_t len;
+ enum bpf_prog_type prog_type;
+} section_names[] = {
+ BPF_PROG_SEC("socket", BPF_PROG_TYPE_SOCKET_FILTER),
+ BPF_PROG_SEC("kprobe/", BPF_PROG_TYPE_KPROBE),
+ BPF_PROG_SEC("kretprobe/", BPF_PROG_TYPE_KPROBE),
+ BPF_PROG_SEC("tracepoint/", BPF_PROG_TYPE_TRACEPOINT),
+ BPF_PROG_SEC("xdp", BPF_PROG_TYPE_XDP),
+ BPF_PROG_SEC("perf_event", BPF_PROG_TYPE_PERF_EVENT),
+ BPF_PROG_SEC("cgroup/skb", BPF_PROG_TYPE_CGROUP_SKB),
+ BPF_PROG_SEC("cgroup/sock", BPF_PROG_TYPE_CGROUP_SOCK),
+ BPF_PROG_SEC("cgroup/dev", BPF_PROG_TYPE_CGROUP_DEVICE),
+ BPF_PROG_SEC("sockops", BPF_PROG_TYPE_SOCK_OPS),
+ BPF_PROG_SEC("sk_skb", BPF_PROG_TYPE_SK_SKB),
+};
+#undef BPF_PROG_SEC
+
+static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
+{
+ int i;
+
+ if (!prog->section_name)
+ goto err;
+
+ for (i = 0; i < ARRAY_SIZE(section_names); i++)
+ if (strncmp(prog->section_name, section_names[i].sec,
+ section_names[i].len) == 0)
+ return section_names[i].prog_type;
+
+err:
+ pr_warning("failed to guess program type based on section name %s\n",
+ prog->section_name);
+
+ return BPF_PROG_TYPE_UNSPEC;
+}
+
int bpf_map__fd(struct bpf_map *map)
{
return map ? map->fd : -EINVAL;
@@ -1832,6 +1871,18 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type,
return -ENOENT;
}
+ /*
+ * If type is not specified, try to guess it based on
+ * section name.
+ */
+ if (type == BPF_PROG_TYPE_UNSPEC) {
+ type = bpf_program__guess_type(prog);
+ if (type == BPF_PROG_TYPE_UNSPEC) {
+ bpf_object__close(obj);
+ return -EINVAL;
+ }
+ }
+
bpf_program__set_type(prog, type);
err = bpf_object__load(obj);
if (err) {
--
2.14.3
^ permalink raw reply related
* [PATCH v3 net-next 3/4] bpftool: implement prog load command
From: Roman Gushchin @ 2017-12-08 14:52 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171208145236.12635-1-guro@fb.com>
Add the prog load command to load a bpf program from a specified
binary file and pin it to bpffs.
Usage description and examples are given in the corresponding man
page.
Syntax:
$ bpftool prog load OBJ FILE
FILE is a non-existing file on bpffs.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 10 +++-
tools/bpf/bpftool/Documentation/bpftool.rst | 2 +-
tools/bpf/bpftool/common.c | 71 +++++++++++++-----------
tools/bpf/bpftool/main.h | 1 +
tools/bpf/bpftool/prog.c | 29 +++++++++-
5 files changed, 79 insertions(+), 34 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 36e8d1c3c40d..ffdb20e8280f 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -15,7 +15,7 @@ SYNOPSIS
*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] | { **-f** | **--bpffs** } }
*COMMANDS* :=
- { **show** | **dump xlated** | **dump jited** | **pin** | **help** }
+ { **show** | **dump xlated** | **dump jited** | **pin** | **load** | **help** }
MAP COMMANDS
=============
@@ -24,6 +24,7 @@ MAP COMMANDS
| **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog dump jited** *PROG* [{**file** *FILE* | **opcodes**}]
| **bpftool** **prog pin** *PROG* *FILE*
+| **bpftool** **prog load** *OBJ* *FILE*
| **bpftool** **prog help**
|
| *PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
@@ -57,6 +58,11 @@ DESCRIPTION
Note: *FILE* must be located in *bpffs* mount.
+ **bpftool prog load** *OBJ* *FILE*
+ Load bpf program from binary *OBJ* and pin as *FILE*.
+
+ Note: *FILE* must be located in *bpffs* mount.
+
**bpftool prog help**
Print short help message.
@@ -126,8 +132,10 @@ EXAMPLES
|
| **# mount -t bpf none /sys/fs/bpf/**
| **# bpftool prog pin id 10 /sys/fs/bpf/prog**
+| **# bpftool prog load ./my_prog.o /sys/fs/bpf/prog2**
| **# ls -l /sys/fs/bpf/**
| -rw------- 1 root root 0 Jul 22 01:43 prog
+| -rw------- 1 root root 0 Jul 22 01:44 prog2
**# bpftool prog dum jited pinned /sys/fs/bpf/prog opcodes**
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index 926c03d5a8da..f547a0c0aa34 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -26,7 +26,7 @@ SYNOPSIS
| **pin** | **help** }
*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
- | **help** }
+ | **load** | **help** }
DESCRIPTION
===========
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 2bd3b280e6dd..b62c94e3997a 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -163,13 +163,49 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
return fd;
}
-int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
+int do_pin_fd(int fd, const char *name)
{
char err_str[ERR_MAX_LEN];
- unsigned int id;
- char *endptr;
char *file;
char *dir;
+ int err = 0;
+
+ err = bpf_obj_pin(fd, name);
+ if (!err)
+ goto out;
+
+ file = malloc(strlen(name) + 1);
+ strcpy(file, name);
+ dir = dirname(file);
+
+ if (errno != EPERM || is_bpffs(dir)) {
+ p_err("can't pin the object (%s): %s", name, strerror(errno));
+ goto out_free;
+ }
+
+ /* Attempt to mount bpffs, then retry pinning. */
+ err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
+ if (!err) {
+ err = bpf_obj_pin(fd, name);
+ if (err)
+ p_err("can't pin the object (%s): %s", name,
+ strerror(errno));
+ } else {
+ err_str[ERR_MAX_LEN - 1] = '\0';
+ p_err("can't mount BPF file system to pin the object (%s): %s",
+ name, err_str);
+ }
+
+out_free:
+ free(file);
+out:
+ return err;
+}
+
+int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
+{
+ unsigned int id;
+ char *endptr;
int err;
int fd;
@@ -195,35 +231,8 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
return -1;
}
- err = bpf_obj_pin(fd, *argv);
- if (!err)
- goto out_close;
-
- file = malloc(strlen(*argv) + 1);
- strcpy(file, *argv);
- dir = dirname(file);
-
- if (errno != EPERM || is_bpffs(dir)) {
- p_err("can't pin the object (%s): %s", *argv, strerror(errno));
- goto out_free;
- }
+ err = do_pin_fd(fd, *argv);
- /* Attempt to mount bpffs, then retry pinning. */
- err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
- if (!err) {
- err = bpf_obj_pin(fd, *argv);
- if (err)
- p_err("can't pin the object (%s): %s", *argv,
- strerror(errno));
- } else {
- err_str[ERR_MAX_LEN - 1] = '\0';
- p_err("can't mount BPF file system to pin the object (%s): %s",
- *argv, err_str);
- }
-
-out_free:
- free(file);
-out_close:
close(fd);
return err;
}
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index bff330b49791..bec1ccbb49c7 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -111,6 +111,7 @@ char *get_fdinfo(int fd, const char *key);
int open_obj_pinned(char *path);
int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
+int do_pin_fd(int fd, const char *name);
int do_prog(int argc, char **arg);
int do_map(int argc, char **arg);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index ad619b96c276..037484ceaeaf 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -45,6 +45,7 @@
#include <sys/stat.h>
#include <bpf.h>
+#include <libbpf.h>
#include "main.h"
#include "disasm.h"
@@ -635,6 +636,30 @@ static int do_pin(int argc, char **argv)
return err;
}
+static int do_load(int argc, char **argv)
+{
+ struct bpf_object *obj;
+ int prog_fd;
+
+ if (argc != 2)
+ usage();
+
+ if (bpf_prog_load(argv[0], BPF_PROG_TYPE_UNSPEC, &obj, &prog_fd)) {
+ p_err("failed to load program\n");
+ return -1;
+ }
+
+ if (do_pin_fd(prog_fd, argv[1])) {
+ p_err("failed to pin program\n");
+ return -1;
+ }
+
+ if (json_output)
+ jsonw_null(json_wtr);
+
+ return 0;
+}
+
static int do_help(int argc, char **argv)
{
if (json_output) {
@@ -647,13 +672,14 @@ static int do_help(int argc, char **argv)
" %s %s dump xlated PROG [{ file FILE | opcodes }]\n"
" %s %s dump jited PROG [{ file FILE | opcodes }]\n"
" %s %s pin PROG FILE\n"
+ " %s %s load OBJ FILE\n"
" %s %s help\n"
"\n"
" " HELP_SPEC_PROGRAM "\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
- bin_name, argv[-2], bin_name, argv[-2]);
+ bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
return 0;
}
@@ -663,6 +689,7 @@ static const struct cmd cmds[] = {
{ "help", do_help },
{ "dump", do_dump },
{ "pin", do_pin },
+ { "load", do_load },
{ 0 }
};
--
2.14.3
^ permalink raw reply related
* [PATCH v3 net-next 2/4] libbpf: prefer global symbols as bpf program name source
From: Roman Gushchin @ 2017-12-08 14:52 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
In-Reply-To: <20171208145236.12635-1-guro@fb.com>
Libbpf picks the name of the first symbol in the corresponding
elf section to use as a program name. But without taking symbol's
scope into account it may end's up with some local label
as a program name. E.g.:
$ bpftool prog
1: type 15 name LBB0_10 tag 0390a5136ba23f5c
loaded_at Dec 07/17:22 uid 0
xlated 456B not jited memlock 4096B
Fix this by preferring global symbols as program name.
For instance:
$ bpftool prog
1: type 15 name bpf_prog1 tag 0390a5136ba23f5c
loaded_at Dec 07/17:26 uid 0
xlated 456B not jited memlock 4096B
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>
Cc: David Ahern <dsahern@gmail.com>
---
tools/lib/bpf/libbpf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 205b7822fa0a..65d0d0aff4fa 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -387,6 +387,8 @@ bpf_object__init_prog_names(struct bpf_object *obj)
continue;
if (sym.st_shndx != prog->idx)
continue;
+ if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL)
+ continue;
name = elf_strptr(obj->efile.elf,
obj->efile.strtabidx,
--
2.14.3
^ permalink raw reply related
* [PATCH v3 net-next 0/4] bpftool: cgroup bpf operations
From: Roman Gushchin @ 2017-12-08 14:52 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai,
guro, Quentin Monnet, David Ahern
This patchset adds basic cgroup bpf operations to bpftool.
Right now there is no convenient way to perform these operations.
The /samples/bpf/load_sock_ops.c implements attach/detacg operations,
but only for BPF_CGROUP_SOCK_OPS programs. Bps (part of bcc) implements
bpf introspection, but lacks any cgroup-related specific.
I find having a tool to perform these basic operations in the kernel tree
very useful, as it can be used in the corresponding bpf documentation
without creating additional dependencies. And bpftool seems to be
a right tool to extend with such functionality.
v3:
- SRC replaced with OBJ in prog load docs
- Output unknown attach type in hex
- License header in SPDX format
- Minor style fixes (e.g. variable reordering)
v2:
- Added prog load operations
- All cgroup operations are looking like bpftool cgroup <command>
- All cgroup-related stuff is moved to a separate file
- Added support for attach flags
- Added support for attaching/detaching programs by id, pinned name, etc
- Changed cgroup detach arguments order
- Added empty json output for succesful programs
- Style fixed: includes order, strncmp and macroses, error handling
- Added man pages
v1:
https://lwn.net/Articles/740366/
Roman Gushchin (4):
libbpf: add ability to guess program type based on section name
libbpf: prefer global symbols as bpf program name source
bpftool: implement prog load command
bpftool: implement cgroup bpf operations
tools/bpf/bpftool/Documentation/bpftool-cgroup.rst | 92 +++++++
tools/bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 12 +-
tools/bpf/bpftool/Documentation/bpftool.rst | 8 +-
tools/bpf/bpftool/cgroup.c | 300 +++++++++++++++++++++
tools/bpf/bpftool/common.c | 71 ++---
tools/bpf/bpftool/main.c | 3 +-
tools/bpf/bpftool/main.h | 2 +
tools/bpf/bpftool/prog.c | 29 +-
tools/lib/bpf/libbpf.c | 53 ++++
10 files changed, 533 insertions(+), 39 deletions(-)
create mode 100644 tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
create mode 100644 tools/bpf/bpftool/cgroup.c
--
2.14.3
^ permalink raw reply
* [PATCH net-next] macvlan: fix memory hole in macvlan_dev
From: Girish Moodalbail @ 2017-12-08 14:03 UTC (permalink / raw)
To: netdev, davem
Move 'macaddr_count' from after 'netpoll' to after 'nest_level' to pack
and reduce a memory hole.
Fixes: 88ca59d1aaf28c25 (macvlan: remove unused fields in struct macvlan_dev)
Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
---
include/linux/if_macvlan.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index bedf54b..4cb7aee 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -30,10 +30,10 @@ struct macvlan_dev {
enum macvlan_mode mode;
u16 flags;
int nest_level;
+ unsigned int macaddr_count;
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *netpoll;
#endif
- unsigned int macaddr_count;
};
static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v3] leds: trigger: Introduce a NETDEV trigger
From: Philippe Ombredanne @ 2017-12-08 14:32 UTC (permalink / raw)
To: Pavel Machek
Cc: Ben Whitten, rpurdie, jacek.anaszewski, linux-leds, LKML, netdev
In-Reply-To: <20171208142707.GA7793@amd>
Pavel,
On Fri, Dec 8, 2017 at 3:27 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Thu 2017-12-07 14:01:39, Philippe Ombredanne wrote:
>> Ben,
>>
>> On Thu, Dec 7, 2017 at 12:46 PM, Ben Whitten <ben.whitten@gmail.com> wrote:
>> > From: Ben Whitten <ben.whitten@gmail.com>
>> >
>> > This commit introduces a NETDEV trigger for named device
>> > activity. Available triggers are link, rx, and tx.
>> >
>> > Signed-off-by: Ben Whitten <ben.whitten@gmail.com>
>> []
>> > --- /dev/null
>> > +++ b/drivers/leds/trigger/ledtrig-netdev.c
>> > @@ -0,0 +1,503 @@
>> > +/*
>> > + * LED Kernel Netdev Trigger
>> > + *
>> > + * Toggles the LED to reflect the link and traffic state of a named net device
>> > + *
>> > + * Copyright 2017 Ben Whitten <ben.whitten@gmail.com>
>> > + *
>> > + * Copyright 2007 Oliver Jowett <oliver@opencloud.com>
>> > + *
>> > + * Derived from ledtrig-timer.c which is:
>> > + * Copyright 2005-2006 Openedhand Ltd.
>> > + * Author: Richard Purdie <rpurdie@openedhand.com>
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + *
>> > + */
>>
>> Have you considered using the new SPDX id instead ? See Thomas doc
>> patches and Greg and Linus comments on the topic
>> Here this would likely come out this way (yes, using a C++ comment at the top):
>>
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +/*
>> > + * LED Kernel Netdev Trigger
>> > + *
>> > + * Toggles the LED to reflect the link and traffic state of a named net device
>> > + *
>> > + * Copyright 2017 Ben Whitten <ben.whitten@gmail.com>
>> > + *
>> > + * Copyright 2007 Oliver Jowett <oliver@opencloud.com>
>> > + *
>> > + * Derived from ledtrig-timer.c which is:
>> > + * Copyright 2005-2006 Openedhand Ltd.
>> > + * Author: Richard Purdie <rpurdie@openedhand.com>
>> > + *
>> > + */
>>
>>
>> This is cleaner and simpler, don't you think?
>
> Please consider putting SPDX where it logically belongs -- near the
> copyright.
Linus wants it on the first line. So you could write this way alright:
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +// Copyright 2017 Ben Whitten <ben.whitten@gmail.com>
>> > +// Copyright 2007 Oliver Jowett <oliver@opencloud.com>
>> > +/*
>> > + * LED Kernel Netdev Trigger
>> > + *
>> > + * Toggles the LED to reflect the link and traffic state of a named net device
>> > + *
>> > + * Derived from ledtrig-timer.c which is:
>> > + * Copyright 2005-2006 Openedhand Ltd.
>> > + * Author: Richard Purdie <rpurdie@openedhand.com>
>> > + *
>> > + */
For reference please check Linus [1][2][3], Thomas [4] and Greg [5]
comments on the topic of C++ style // comments and first line placement.
Jonathan also wrote a nice background article on the SPDXification
topic at LWN [6]
[1] https://lkml.org/lkml/2017/11/2/715
[2] https://lkml.org/lkml/2017/11/25/125
[3] https://lkml.org/lkml/2017/11/25/133
[4] https://lkml.org/lkml/2017/11/2/805
[5] https://lkml.org/lkml/2017/10/19/165
[6] https://lwn.net/Articles/739183/
--
Cordially
Philippe Ombredanne
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox