* Re: Performance regressions in TCP_STREAM tests in Linux 4.15 (and later)
From: Steven Rostedt @ 2018-04-30 16:31 UTC (permalink / raw)
To: Ben Greear
Cc: Michael Wenig, netdev@vger.kernel.org, eric.dumazet@gmail.com,
Shilpi Agarwal, Boon Ang, Darren Hart, Abdul Anshad Azeez
In-Reply-To: <476bfc0f-eb2d-fe57-73d9-ec8a8392ad33@candelatech.com>
On Mon, 30 Apr 2018 09:14:04 -0700
Ben Greear <greearb@candelatech.com> wrote:
> >> As part of VMware's performance testing with the Linux 4.15 kernel,
> >> we identified CPU cost and throughput regressions when comparing to
> >> the Linux 4.14 kernel. The impacted test cases are mostly TCP_STREAM
> >> send tests when using small message sizes. The regressions are
> >> significant (up 3x) and were tracked down to be a side effect of Eric
> >> Dumazat's RB tree changes that went into the Linux 4.15 kernel.
> >> Further investigation showed our use of the TCP_NODELAY flag in
> >> conjunction with Eric's change caused the regressions to show and
> >> simply disabling TCP_NODELAY brought performance back to normal.
> >> Eric's change also resulted into significant improvements in our
> >> TCP_RR test cases.
> >>
> >>
> >>
> >> Based on these results, our theory is that Eric's change made the
> >> system overall faster (reduced latency) but as a side effect less
> >> aggregation is happening (with TCP_NODELAY) and that results in lower
> >> throughput. Previously even though TCP_NODELAY was set, system was
> >> slower and we still got some benefit of aggregation. Aggregation
> >> helps in better efficiency and higher throughput although it can
> >> increase the latency. If you are seeing a regression in your
> >> application throughput after this change, using TCP_NODELAY might
> >> help bring performance back however that might increase latency.
>
> I guess you mean _disabling_ TCP_NODELAY instead of _using_ TCP_NODELAY?
Yes, thank you for catching that.
-- Steve
^ permalink raw reply
* Re: [RFC net-next 0/5] Support for PHY test modes
From: Florian Fainelli @ 2018-04-30 16:30 UTC (permalink / raw)
To: David Miller
Cc: netdev, andrew, rmk, linux-kernel, cphealy, nikita.yoush,
vivien.didelot, Nisar.Sayed, UNGLinuxDriver
In-Reply-To: <20180429.225554.2165914401649980919.davem@davemloft.net>
On 04/29/2018 07:55 PM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Fri, 27 Apr 2018 17:32:30 -0700
>
>> This patch series adds support for specifying PHY test modes through
>> ethtool and paves the ground for adding support for more complex
>> test modes that might require data to be exchanged between user and
>> kernel space.
>>
>> As an example, patches are included to add support for the IEEE
>> electrical test modes for 100BaseT2 and 1000BaseT. Those do not
>> require data to be passed back and forth.
>>
>> I believe the infrastructure to be usable enough to add support for
>> other things like:
>>
>> - cable diagnostics
>> - pattern generator/waveform generator with specific pattern being
>> indicated for instance
>>
>> Questions for Andrew, and others:
>>
>> - there could be room for adding additional ETH_TEST_FL_* values in order to
>> help determine how the test should be running
>> - some of these tests can be disruptive to connectivity, the minimum we could
>> do is stop the PHY state machine and restart it when "normal" is used to exit
>> those test modes
>>
>> Comments welcome!
>
> Generally, no objection to providing this in the general manner you
> have implemented it via ethtool.
Thanks for taking a look!
>
> I think in order to answer the disruptive question, you need to give
> some information about what kind of context this stuff would be
> used in, and if in those contexts what the user expectations are
> or might be.
>
> Are these test modes something that usually would be initiated with
> the interface down?
We expect that these commands/tests would likely be issued when the
interface is up (not necessarily with a carrier state ON though) because
we know for sure that drivers will have successfully connected to their
PHY and there is no power management (or there is, like runtime PM)
which will not prevent accesses to the MDIO interface from working.
Turning these tests on will typically result in the link partner
dropping the link with us, and the interface will be non-functional as
far as the data path is concerned (similar to an isolation mode). This
might warrant properly reporting that to user-space through e.g: a
private IFF_* value maybe?
--
Florian
^ permalink raw reply
* [PATCH] net/mlx4: fix spelling mistake: "failedi" -> "failed"
From: Colin King @ 2018-04-30 16:29 UTC (permalink / raw)
To: Tariq Toukan, David S . Miller, netdev, linux-rdma
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
trivial fix to spelling mistake in mlx4_warn message.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index bfef69235d71..211578ffc70d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -1317,7 +1317,7 @@ static int mlx4_mf_unbond(struct mlx4_dev *dev)
ret = mlx4_unbond_fs_rules(dev);
if (ret)
- mlx4_warn(dev, "multifunction unbond for flow rules failedi (%d)\n", ret);
+ mlx4_warn(dev, "multifunction unbond for flow rules failed (%d)\n", ret);
ret1 = mlx4_unbond_mac_table(dev);
if (ret1) {
mlx4_warn(dev, "multifunction unbond for MAC table failed (%d)\n", ret1);
--
2.17.0
^ permalink raw reply related
* Re: Performance regressions in TCP_STREAM tests in Linux 4.15 (and later)
From: Ben Greear @ 2018-04-30 16:14 UTC (permalink / raw)
To: Steven Rostedt, Michael Wenig
Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com, Shilpi Agarwal,
Boon Ang, Darren Hart, Steven Rostedt, Abdul Anshad Azeez
In-Reply-To: <20180427231149.119db14c@vmware.local.home>
On 04/27/2018 08:11 PM, Steven Rostedt wrote:
>
> We'd like this email archived in netdev list, but since netdev is
> notorious for blocking outlook email as spam, it didn't go through. So
> I'm replying here to help get it into the archives.
>
> Thanks!
>
> -- Steve
>
>
> On Fri, 27 Apr 2018 23:05:46 +0000
> Michael Wenig <mwenig@vmware.com> wrote:
>
>> As part of VMware's performance testing with the Linux 4.15 kernel,
>> we identified CPU cost and throughput regressions when comparing to
>> the Linux 4.14 kernel. The impacted test cases are mostly TCP_STREAM
>> send tests when using small message sizes. The regressions are
>> significant (up 3x) and were tracked down to be a side effect of Eric
>> Dumazat's RB tree changes that went into the Linux 4.15 kernel.
>> Further investigation showed our use of the TCP_NODELAY flag in
>> conjunction with Eric's change caused the regressions to show and
>> simply disabling TCP_NODELAY brought performance back to normal.
>> Eric's change also resulted into significant improvements in our
>> TCP_RR test cases.
>>
>>
>>
>> Based on these results, our theory is that Eric's change made the
>> system overall faster (reduced latency) but as a side effect less
>> aggregation is happening (with TCP_NODELAY) and that results in lower
>> throughput. Previously even though TCP_NODELAY was set, system was
>> slower and we still got some benefit of aggregation. Aggregation
>> helps in better efficiency and higher throughput although it can
>> increase the latency. If you are seeing a regression in your
>> application throughput after this change, using TCP_NODELAY might
>> help bring performance back however that might increase latency.
I guess you mean _disabling_ TCP_NODELAY instead of _using_ TCP_NODELAY?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [net-next v2] ipv6: sr: extract the right key values for "seg6_make_flowlabel"
From: David Miller @ 2018-04-30 16:13 UTC (permalink / raw)
To: amsalam20; +Cc: dav.lebrun, netdev, linux-kernel
In-Reply-To: <1524910715-12097-1-git-send-email-amsalam20@gmail.com>
From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Sat, 28 Apr 2018 12:18:35 +0200
> The seg6_make_flowlabel() is used by seg6_do_srh_encap() to compute the
> flowlabel from a given skb. It relies on skb_get_hash() which eventually
> calls __skb_flow_dissect() to extract the flow_keys struct values from
> the skb.
>
> In case of IPv4 traffic, calling seg6_make_flowlabel() after skb_push(),
> skb_reset_network_header(), and skb_mac_header_rebuild() will results in
> flow_keys struct of all key values set to zero.
>
> This patch calls seg6_make_flowlabel() before resetting the headers of skb
> to get the right key values.
>
> Extracted Key values are based on the type inner packet as follows:
> 1) IPv6 traffic: src_IP, dst_IP, L4 proto, and flowlabel of inner packet.
> 2) IPv4 traffic: src_IP, dst_IP, L4 proto, src_port, and dst_port
> 3) L2 traffic: depends on what kind of traffic carried into the L2
> frame. IPv6 and IPv4 traffic works as discussed 1) and 2)
>
> Here a hex_dump of struct flow_keys for IPv4 and IPv6 traffic
> 10.100.1.100: 47302 > 30.0.0.2: 5001
> 00000000: 14 00 02 00 00 00 00 00 08 00 11 00 00 00 00 00
> 00000010: 00 00 00 00 00 00 00 00 13 89 b8 c6 1e 00 00 02
> 00000020: 0a 64 01 64
>
> fc00:a1:a > b2::2
> 00000000: 28 00 03 00 00 00 00 00 86 dd 11 00 99 f9 02 00
> 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 b2 00 00
> 00000020: 00 00 00 00 00 00 00 00 00 00 00 02 fc 00 00 a1
> 00000030: 00 00 00 00 00 00 00 00 00 00 00 0a
>
> Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
Looks good, applied, thank you.
^ permalink raw reply
* Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
From: David Miller @ 2018-04-30 16:10 UTC (permalink / raw)
To: eric.dumazet
Cc: soheil.kdev, netdev, ycheng, ncardwell, edumazet, willemb, soheil
In-Reply-To: <c95883da-51ab-47aa-7ad1-a5bb85935e6b@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Apr 2018 09:01:47 -0700
> TCP sockets are read by a single thread really (or synchronized
> threads), or garbage is ensured, regardless of how the kernel
> ensures locking while reporting "queue length"
Whatever applications "typically do", we should never return
garbage, and that is what this code allowing to happen.
Everything else in recvmsg() operates on state under the proper socket
lock, to ensure consistency.
The only reason we are releasing the socket lock first it to make sure
the backlog is processed and we have the most update information
available.
It seems like one is striving for correctness and better accuracy, no?
:-)
Look, this can be fixed really simply. And if you are worried about
unbounded loops if two apps maliciously do recvmsg() in parallel,
then don't even loop, just fallback to full socket locking and make
the "non-typical" application pay the price:
tmp1 = A;
tmp2 = B;
barrier();
tmp3 = A;
if (unlikely(tmp1 != tmp3)) {
lock_sock(sk);
tmp1 = A;
tmp2 = B;
release_sock(sk);
}
I'm seriously not applying the patch as-is, sorry. This issue
must be addressed somehow.
Thank you.
^ permalink raw reply
* Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
From: Eric Dumazet @ 2018-04-30 16:01 UTC (permalink / raw)
To: David Miller, eric.dumazet
Cc: soheil.kdev, netdev, ycheng, ncardwell, edumazet, willemb, soheil
In-Reply-To: <20180430.115605.1094351453502803017.davem@davemloft.net>
On 04/30/2018 08:56 AM, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 30 Apr 2018 08:43:50 -0700
>
>> I say sort of, because by the time we have any number, TCP might
>> have received more packets anyway.
>
> That's fine.
>
> However, the number reported should have been true at least at some
> finite point in time.
>
> If you allow overlapping changes to either of the two variables during
> the sampling, then you are reporting a number which was never true at
> any point in time.
>
> It is essentially garbage.
Correct.
TCP sockets are read by a single thread really (or synchronized threads),
or garbage is ensured, regardless of how the kernel ensures locking while reporting "queue length"
^ permalink raw reply
* Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
From: Soheil Hassas Yeganeh @ 2018-04-30 15:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Yuchung Cheng, Neal Cardwell, Eric Dumazet,
Willem de Bruijn
In-Reply-To: <aec45003-3354-e49f-b032-5297e98722eb@gmail.com>
On Mon, Apr 30, 2018 at 11:43 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 04/30/2018 08:38 AM, David Miller wrote:
>> From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
>> Date: Fri, 27 Apr 2018 14:57:32 -0400
>>
>>> Since the socket lock is not held when calculating the size of
>>> receive queue, TCP_INQ is a hint. For example, it can overestimate
>>> the queue size by one byte, if FIN is received.
>>
>> I think it is even worse than that.
>>
>> If another application comes in and does a recvmsg() in parallel with
>> these calculations, you could even report a negative value.
Thanks you David. In addition to Eric's point, for TCP specifically,
it is quite uncommon to have multiple threads calling recvmsg() for
the same socket in parallel, because the application is interested in
the streamed, in-sequence bytes. Except when the application just
wants to discard the incoming stream or has a predefined frame sizes,
this wouldn't be an issue. For such cases, the proposed INQ hint is
not going to be useful.
Could you please let me know whether you have any other example in mind?
Thanks!
Soheil
^ permalink raw reply
* [PATCH bpf-next] bpf: relax constraints on formatting for eBPF helper documentation
From: Quentin Monnet @ 2018-04-30 15:59 UTC (permalink / raw)
To: daniel, ast; +Cc: dsahern, yhs, netdev, oss-drivers, quentin.monnet
The Python script used to parse and extract eBPF helpers documentation
from include/uapi/linux/bpf.h expects a very specific formatting for the
descriptions (single dots represent a space, '>' stands for a tab):
/*
...
*.int bpf_helper(list of arguments)
*.> Description
*.> > Start of description
*.> > Another line of description
*.> > And yet another line of description
*.> Return
*.> > 0 on success, or a negative error in case of failure
...
*/
This is too strict, and painful for developers who wants to add
documentation for new helpers. Worse, it is extremelly difficult to
check that the formatting is correct during reviews. Change the
format expected by the script and make it more flexible. The script now
works whether or not the initial space (right after the star) is
present, and accepts both tabs and white spaces (or a combination of
both) for indenting description sections and contents.
Concretely, something like the following would now be supported:
/*
...
*int bpf_helper(list of arguments)
*......Description
*.> > Start of description...
*> > Another line of description
*..............And yet another line of description
*> Return
*.> ........0 on success, or a negative error in case of failure
...
*/
Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
scripts/bpf_helpers_doc.py | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index 30ba0fee36e4..717547e6f0a6 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -87,7 +87,7 @@ class HeaderParser(object):
# - Same as above, with "const" and/or "struct" in front of type
# - "..." (undefined number of arguments, for bpf_trace_printk())
# There is at least one term ("void"), and at most five arguments.
- p = re.compile('^ \* ((.+) \**\w+\((((const )?(struct )?(\w+|\.\.\.)( \**\w+)?)(, )?){1,5}\))$')
+ p = re.compile('^ \* ?((.+) \**\w+\((((const )?(struct )?(\w+|\.\.\.)( \**\w+)?)(, )?){1,5}\))$')
capture = p.match(self.line)
if not capture:
raise NoHelperFound
@@ -95,7 +95,7 @@ class HeaderParser(object):
return capture.group(1)
def parse_desc(self):
- p = re.compile('^ \* \tDescription$')
+ p = re.compile('^ \* ?(?:\t| {6,8})Description$')
capture = p.match(self.line)
if not capture:
# Helper can have empty description and we might be parsing another
@@ -109,7 +109,7 @@ class HeaderParser(object):
if self.line == ' *\n':
desc += '\n'
else:
- p = re.compile('^ \* \t\t(.*)')
+ p = re.compile('^ \* ?(?:\t| {6,8})(?:\t| {8})(.*)')
capture = p.match(self.line)
if capture:
desc += capture.group(1) + '\n'
@@ -118,7 +118,7 @@ class HeaderParser(object):
return desc
def parse_ret(self):
- p = re.compile('^ \* \tReturn$')
+ p = re.compile('^ \* ?(?:\t| {6,8})Return$')
capture = p.match(self.line)
if not capture:
# Helper can have empty retval and we might be parsing another
@@ -132,7 +132,7 @@ class HeaderParser(object):
if self.line == ' *\n':
ret += '\n'
else:
- p = re.compile('^ \* \t\t(.*)')
+ p = re.compile('^ \* ?(?:\t| {6,8})(?:\t| {8})(.*)')
capture = p.match(self.line)
if capture:
ret += capture.group(1) + '\n'
--
2.14.1
^ permalink raw reply related
* Re: [PATCH bpf-next] tools include uapi: Grab a copy of linux/erspan.h
From: Daniel Borkmann @ 2018-04-30 15:57 UTC (permalink / raw)
To: Y Song; +Cc: William Tu, netdev, Yonghong Song
In-Reply-To: <CAH3MdRV6kR=4G0nqJxUWMP72SPE7hVaSHVebMOVML33O0c1A9w@mail.gmail.com>
On 04/30/2018 05:45 PM, Y Song wrote:
> On Mon, Apr 30, 2018 at 7:33 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 04/30/2018 04:26 PM, William Tu wrote:
>>> Bring the erspan uapi header file so BPF tunnel helpers can use it.
>>>
>>> Fixes: 933a741e3b82 ("selftests/bpf: bpf tunnel test.")
>>> Reported-by: Yonghong Song <yhs@fb.com>
>>> Signed-off-by: William Tu <u9012063@gmail.com>
>>
>> Thanks for the patch, William! I also Cc'ed Yonghong here, so he has a
>> chance to try it out.
>
> Just tried it out. It works. Thanks for fixing!
> Acked-by: Yonghong Song <yhs@fb.com>
Applied to bpf-next, thanks everyone!
^ permalink raw reply
* Re: [PATCH] change the comment of vti6_ioctl
From: David Miller @ 2018-04-30 15:57 UTC (permalink / raw)
To: sunlw.fnst; +Cc: netdev, steffen.klassert, herbert
In-Reply-To: <20180429070552.2472-1-sunlw.fnst@cn.fujitsu.com>
From: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
Date: Sun, 29 Apr 2018 15:05:52 +0800
> The comment of vti6_ioctl() is wrong. which use vti6_tnl_ioctl
> instead of vti6_ioctl.
>
> Signed-off-by: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
Please CC: the IPSEC maintainers on future patch submissions to IPSEC
files, as per the top level MAINTAINERS file.
Steffen, please queue this up, thank you.
> ---
> net/ipv6/ip6_vti.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> index c214ffec02f0..deadc4c3703b 100644
> --- a/net/ipv6/ip6_vti.c
> +++ b/net/ipv6/ip6_vti.c
> @@ -743,7 +743,7 @@ vti6_parm_to_user(struct ip6_tnl_parm2 *u, const struct __ip6_tnl_parm *p)
> }
>
> /**
> - * vti6_tnl_ioctl - configure vti6 tunnels from userspace
> + * vti6_ioctl - configure vti6 tunnels from userspace
> * @dev: virtual device associated with tunnel
> * @ifr: parameters passed from userspace
> * @cmd: command to be performed
> --
> 2.17.0
>
>
>
^ permalink raw reply
* Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
From: David Miller @ 2018-04-30 15:56 UTC (permalink / raw)
To: eric.dumazet
Cc: soheil.kdev, netdev, ycheng, ncardwell, edumazet, willemb, soheil
In-Reply-To: <aec45003-3354-e49f-b032-5297e98722eb@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Apr 2018 08:43:50 -0700
> I say sort of, because by the time we have any number, TCP might
> have received more packets anyway.
That's fine.
However, the number reported should have been true at least at some
finite point in time.
If you allow overlapping changes to either of the two variables during
the sampling, then you are reporting a number which was never true at
any point in time.
It is essentially garbage.
^ permalink raw reply
* Re: [PATCH net-next 0/2 v5] netns: uevent filtering
From: Eric W. Biederman @ 2018-04-30 15:55 UTC (permalink / raw)
To: Christian Brauner
Cc: davem, netdev, linux-kernel, avagin, ktkhai, serge, gregkh
In-Reply-To: <20180429104412.22445-1-christian.brauner@ubuntu.com>
Christian Brauner <christian.brauner@ubuntu.com> writes:
> Hey everyone,
>
> This is the new approach to uevent filtering as discussed (see the
> threads in [1], [2], and [3]). It only contains *non-functional
> changes*.
>
> This series deals with with fixing up uevent filtering logic:
> - uevent filtering logic is simplified
> - locking time on uevent_sock_list is minimized
> - tagged and untagged kobjects are handled in separate codepaths
> - permissions for userspace are fixed for network device uevents in
> network namespaces owned by non-initial user namespaces
> Udev is now able to see those events correctly which it wasn't before.
> For example, moving a physical device into a network namespace not
> owned by the initial user namespaces before gave:
>
> root@xen1:~# udevadm --debug monitor -k
> calling: monitor
> monitor will print the received events for:
> KERNEL - the kernel uevent
>
> sender uid=65534, message ignored
> sender uid=65534, message ignored
> sender uid=65534, message ignored
> sender uid=65534, message ignored
> sender uid=65534, message ignored
>
> and now after the discussion and solution in [3] correctly gives:
>
> root@xen1:~# udevadm --debug monitor -k
> calling: monitor
> monitor will print the received events for:
> KERNEL - the kernel uevent
>
> KERNEL[625.301042] add /devices/pci0000:00/0000:00:02.0/0000:01:00.1/net/enp1s0f1 (net)
> KERNEL[625.301109] move /devices/pci0000:00/0000:00:02.0/0000:01:00.1/net/enp1s0f1 (net)
> KERNEL[625.301138] move /devices/pci0000:00/0000:00:02.0/0000:01:00.1/net/eth1 (net)
> KERNEL[655.333272] remove /devices/pci0000:00/0000:00:02.0/0000:01:00.1/net/eth1 (net)
>
> Thanks!
> Christian
>
> [1]: https://lkml.org/lkml/2018/4/4/739
> [2]: https://lkml.org/lkml/2018/4/26/767
> [3]: https://lkml.org/lkml/2018/4/26/738
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> Christian Brauner (2):
> uevent: add alloc_uevent_skb() helper
> netns: restrict uevents
>
> lib/kobject_uevent.c | 178 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 126 insertions(+), 52 deletions(-)
Eric
^ permalink raw reply
* Re: [PATCH net-next] libcxgb,cxgb4: use __skb_put_zero to simplfy code
From: David Miller @ 2018-04-30 15:54 UTC (permalink / raw)
To: yuehaibing; +Cc: ganeshgr, johannes.berg, netdev, linux-kernel
In-Reply-To: <20180428043522.12408-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Sat, 28 Apr 2018 12:35:22 +0800
> use helper __skb_put_zero to replace the pattern of __skb_put() && memset()
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH bpf-next] tools include uapi: Grab a copy of linux/erspan.h
From: Y Song @ 2018-04-30 15:45 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: William Tu, netdev, Yonghong Song
In-Reply-To: <be743a29-bbf2-1554-a961-0e71ee8ea030@iogearbox.net>
On Mon, Apr 30, 2018 at 7:33 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 04/30/2018 04:26 PM, William Tu wrote:
>> Bring the erspan uapi header file so BPF tunnel helpers can use it.
>>
>> Fixes: 933a741e3b82 ("selftests/bpf: bpf tunnel test.")
>> Reported-by: Yonghong Song <yhs@fb.com>
>> Signed-off-by: William Tu <u9012063@gmail.com>
>
> Thanks for the patch, William! I also Cc'ed Yonghong here, so he has a
> chance to try it out.
Just tried it out. It works. Thanks for fixing!
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* RE: smsc95xx: aligment issues
From: Woojung.Huh @ 2018-04-30 15:44 UTC (permalink / raw)
To: stefan.wahren, Nisar.Sayed
Cc: davem, linux-usb, netdev, popcornmix, james.hughes
In-Reply-To: <455810676.1044.1524902320201@email.1und1.de>
Hi Stefan,
Thanks for report. We will try to repro issue and contact you if need more details.
Regards,
Woojung
> -----Original Message-----
> From: Stefan Wahren [mailto:stefan.wahren@i2se.com]
> Sent: Saturday, April 28, 2018 3:59 AM
> To: Nisar Sayed - I17970 <Nisar.Sayed@microchip.com>; Woojung Huh - C21699
> <Woojung.Huh@microchip.com>
> Cc: David S. Miller <davem@davemloft.net>; linux-usb <linux-usb@vger.kernel.org>; netdev
> <netdev@vger.kernel.org>; popcorn mix <popcornmix@gmail.com>; James Hughes
> <james.hughes@raspberrypi.org>
> Subject: net: smsc95xx: aligment issues
>
> Hi,
> after connecting a Raspberry Pi 1 B to my local network i'm seeing aligment issues under
> /proc/cpu/alignment:
>
> User: 0
> System: 142 (_decode_session4+0x12c/0x3c8)
> Skipped: 0
> Half: 0
> Word: 0
> DWord: 127
> Multi: 15
> User faults: 2 (fixup)
>
> I've also seen outputs with _csum_ipv6_magic.
>
> Kernel config: bcm2835_defconfig
> Reproducible kernel trees: current linux-next, 4.17-rc2 and 4.14.37 (i didn't test older versions)
>
> Please tell if you need more information to narrow down this issue.
>
> Best regards
> Stefan
^ permalink raw reply
* Re: [PATCH net-next] erspan: auto detect truncated packets.
From: David Miller @ 2018-04-30 15:44 UTC (permalink / raw)
To: u9012063; +Cc: netdev
In-Reply-To: <1524863792-66068-1-git-send-email-u9012063@gmail.com>
From: William Tu <u9012063@gmail.com>
Date: Fri, 27 Apr 2018 14:16:32 -0700
> Currently the truncated bit is set only when the mirrored packet
> is larger than mtu. For certain cases, the packet might already
> been truncated before sending to the erspan tunnel. In this case,
> the patch detect whether the IP header's total length is larger
> than the actual skb->len. If true, this indicated that the
> mirrored packet is truncated and set the erspan truncate bit.
>
> I tested the patch using bpf_skb_change_tail helper function to
> shrink the packet size and send to erspan tunnel.
>
> Reported-by: Xiaoyan Jin <xiaoyanj@vmware.com>
> Signed-off-by: William Tu <u9012063@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
From: Eric Dumazet @ 2018-04-30 15:43 UTC (permalink / raw)
To: David Miller, soheil.kdev
Cc: netdev, ycheng, ncardwell, edumazet, willemb, soheil
In-Reply-To: <20180430.113834.1760530542793231849.davem@davemloft.net>
On 04/30/2018 08:38 AM, David Miller wrote:
> From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
> Date: Fri, 27 Apr 2018 14:57:32 -0400
>
>> Since the socket lock is not held when calculating the size of
>> receive queue, TCP_INQ is a hint. For example, it can overestimate
>> the queue size by one byte, if FIN is received.
>
> I think it is even worse than that.
>
> If another application comes in and does a recvmsg() in parallel with
> these calculations, you could even report a negative value.
>
> These READ_ONCE() make it look like some of these issues are being
> addressed but they are not.
>
> You could freeze the values just by taking sk->sk_lock.slock, but I
> don't know if that cost is considered acceptable or not.
>
> Another idea is to sample both values in a loop, similar to a sequence
> lock sequence:
>
> again:
> tmp1 = A;
> tmp2 = B;
> barrier();
> tmp3 = A;
> if (tmp1 != tmp3)
> goto again;
>
> But the current state of affairs is not going to work well.
>
We want a hint, and max_t(int, 0, ....) does not return a negative value ?
If the hint is wrong in 0.1 % of the cases, we really do not care, it is not meant
to replace the existing precise ( well, sort of ) mechanism.
I say sort of, because by the time we have any number, TCP might have received more packets anyway.
^ permalink raw reply
* Re: KASAN: use-after-free Read in perf_trace_rpc_stats_latency
From: Chuck Lever @ 2018-04-30 15:39 UTC (permalink / raw)
To: syzbot
Cc: Anna Schumaker, Bruce Fields, David S . Miller, Jeff Layton, lkml,
Linux NFS Mailing List, netdev, syzkaller-bugs, Trond Myklebust
In-Reply-To: <00000000000027a3e2056b10e81e@google.com>
> On Apr 30, 2018, at 9:34 AM, syzbot <syzbot+27db1f90e2b972a5f2d3@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot hit the following crash on bpf-next commit
> f60ad0a0c441530280a4918eca781a6a94dffa50 (Sun Apr 29 15:45:55 2018 +0000)
> Merge branch 'bpf_get_stack'
> syzbot dashboard link: https://syzkaller.appspot.com/bug?extid=27db1f90e2b972a5f2d3
>
> Unfortunately, I don't have any reproducer for this crash yet.
> Raw console output: https://syzkaller.appspot.com/x/log.txt?id=6741221342969856
> Kernel config: https://syzkaller.appspot.com/x/.config?id=4410550353033654931
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+27db1f90e2b972a5f2d3@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for details.
> If you forward the report, please keep this part and the footer.
>
> rpcbind: RPC call returned error 22
> rpcbind: RPC call returned error 22
> rpcbind: RPC call returned error 22
> rpcbind: RPC call returned error 22
> ==================================================================
> BUG: KASAN: use-after-free in strlen+0x83/0xa0 lib/string.c:482
> Read of size 1 at addr ffff8801d6f0a1c0 by task syz-executor7/5079
>
> CPU: 1 PID: 5079 Comm: syz-executor7 Not tainted 4.17.0-rc2+ #16
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1b9/0x294 lib/dump_stack.c:113
> print_address_description+0x6c/0x20b mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
> __asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
> strlen+0x83/0xa0 lib/string.c:482
> trace_event_get_offsets_rpc_stats_latency include/trace/events/sunrpc.h:215 [inline]
> perf_trace_rpc_stats_latency+0x318/0x10d0 include/trace/events/sunrpc.h:215
> trace_rpc_stats_latency include/trace/events/sunrpc.h:215 [inline]
> rpc_count_iostats_metrics+0x594/0x8a0 net/sunrpc/stats.c:182
> rpc_count_iostats+0x76/0x90 net/sunrpc/stats.c:195
> xprt_release+0xa3b/0x1110 net/sunrpc/xprt.c:1351
> rpc_release_resources_task+0x20/0xa0 net/sunrpc/sched.c:1024
> rpc_release_task net/sunrpc/sched.c:1068 [inline]
> __rpc_execute+0x5e9/0xf50 net/sunrpc/sched.c:833
> rpc_execute+0x37f/0x480 net/sunrpc/sched.c:852
> rpc_run_task+0x615/0x8c0 net/sunrpc/clnt.c:1053
> rpc_call_sync+0x196/0x290 net/sunrpc/clnt.c:1082
> rpc_ping+0x155/0x1f0 net/sunrpc/clnt.c:2513
> rpc_create_xprt+0x282/0x3f0 net/sunrpc/clnt.c:479
> rpc_create+0x52e/0x900 net/sunrpc/clnt.c:587
> nfs_create_rpc_client+0x63e/0x850 fs/nfs/client.c:523
> nfs_init_client+0x74/0x100 fs/nfs/client.c:634
> nfs_get_client+0x1065/0x1500 fs/nfs/client.c:425
> nfs_init_server+0x364/0xfb0 fs/nfs/client.c:670
> nfs_create_server+0x86/0x5f0 fs/nfs/client.c:953
> nfs_try_mount+0x177/0xab0 fs/nfs/super.c:1884
> nfs_fs_mount+0x17de/0x2efd fs/nfs/super.c:2695
> mount_fs+0xae/0x328 fs/super.c:1267
> vfs_kern_mount.part.34+0xd4/0x4d0 fs/namespace.c:1037
> vfs_kern_mount fs/namespace.c:1027 [inline]
> do_new_mount fs/namespace.c:2518 [inline]
> do_mount+0x564/0x3070 fs/namespace.c:2848
> ksys_mount+0x12d/0x140 fs/namespace.c:3064
> __do_sys_mount fs/namespace.c:3078 [inline]
> __se_sys_mount fs/namespace.c:3075 [inline]
> __x64_sys_mount+0xbe/0x150 fs/namespace.c:3075
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x455979
> RSP: 002b:00007f1e2785bc68 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
> RAX: ffffffffffffffda RBX: 00007f1e2785c6d4 RCX: 0000000000455979
> RDX: 0000000020fb5ffc RSI: 0000000020343ff8 RDI: 000000002091dff8
> RBP: 000000000072bf50 R08: 000000002000a000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 0000000000000440 R14: 00000000006fa6a0 R15: 0000000000000001
>
> Allocated by task 5079:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
> __do_kmalloc mm/slab.c:3718 [inline]
> __kmalloc_track_caller+0x14a/0x760 mm/slab.c:3733
> kstrdup+0x39/0x70 mm/util.c:56
> xs_format_common_peer_ports+0x130/0x370 net/sunrpc/xprtsock.c:290
> xs_format_peer_addresses net/sunrpc/xprtsock.c:303 [inline]
> xs_setup_udp+0x5ea/0x880 net/sunrpc/xprtsock.c:3037
> xprt_create_transport+0x1d7/0x596 net/sunrpc/xprt.c:1433
> rpc_create+0x489/0x900 net/sunrpc/clnt.c:573
> nfs_create_rpc_client+0x63e/0x850 fs/nfs/client.c:523
> nfs_init_client+0x74/0x100 fs/nfs/client.c:634
> nfs_get_client+0x1065/0x1500 fs/nfs/client.c:425
> nfs_init_server+0x364/0xfb0 fs/nfs/client.c:670
> nfs_create_server+0x86/0x5f0 fs/nfs/client.c:953
> nfs_try_mount+0x177/0xab0 fs/nfs/super.c:1884
> nfs_fs_mount+0x17de/0x2efd fs/nfs/super.c:2695
> mount_fs+0xae/0x328 fs/super.c:1267
> vfs_kern_mount.part.34+0xd4/0x4d0 fs/namespace.c:1037
> vfs_kern_mount fs/namespace.c:1027 [inline]
> do_new_mount fs/namespace.c:2518 [inline]
> do_mount+0x564/0x3070 fs/namespace.c:2848
> ksys_mount+0x12d/0x140 fs/namespace.c:3064
> __do_sys_mount fs/namespace.c:3078 [inline]
> __se_sys_mount fs/namespace.c:3075 [inline]
> __x64_sys_mount+0xbe/0x150 fs/namespace.c:3075
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 6:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
> kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
> __cache_free mm/slab.c:3498 [inline]
> kfree+0xd9/0x260 mm/slab.c:3813
> xs_update_peer_port net/sunrpc/xprtsock.c:309 [inline]
> xs_set_port+0x105/0x180 net/sunrpc/xprtsock.c:1827
> rpcb_getport_done+0x224/0x2d0 net/sunrpc/rpcb_clnt.c:824
> rpc_exit_task+0xc9/0x2d0 net/sunrpc/sched.c:725
> __rpc_execute+0x28a/0xf50 net/sunrpc/sched.c:784
> rpc_async_schedule+0x16/0x20 net/sunrpc/sched.c:857
> process_one_work+0xc1e/0x1b50 kernel/workqueue.c:2145
> worker_thread+0x1cc/0x1440 kernel/workqueue.c:2279
> kthread+0x345/0x410 kernel/kthread.c:238
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
>
> The buggy address belongs to the object at ffff8801d6f0a1c0
> which belongs to the cache kmalloc-32 of size 32
> The buggy address is located 0 bytes inside of
> 32-byte region [ffff8801d6f0a1c0, ffff8801d6f0a1e0)
> The buggy address belongs to the page:
> page:ffffea00075bc280 count:1 mapcount:0 mapping:ffff8801d6f0a000 index:0xffff8801d6f0afc1
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffff8801d6f0a000 ffff8801d6f0afc1 0000000100000024
> raw: ffffea00075c52a0 ffff8801da801238 ffff8801da8001c0 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8801d6f0a080: 01 fc fc fc fc fc fc fc 00 02 fc fc fc fc fc fc
> ffff8801d6f0a100: 00 02 fc fc fc fc fc fc 00 02 fc fc fc fc fc fc
>> ffff8801d6f0a180: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
> ^
> ffff8801d6f0a200: 00 02 fc fc fc fc fc fc 01 fc fc fc fc fc fc fc
> ffff8801d6f0a280: fb fb fb fb fc fc fc fc fb fb fb fb fc fc fc fc
> ==================================================================
The rpc_task survived longer than the transport. task->tk_xprt
points to freed memory by the time rpc_count_iostats_metrics
runs.
The naive fix is to remove the references to task->tk_xprt
in the trace point. I'm still checking to see if this is the
fully correct fix.
> ---
> This bug is generated by a dumb bot. It may contain errors.
> See https://goo.gl/tpsmEJ for details.
> Direct all questions to syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new bug report.
> Note: all commands must start from beginning of the line in the email body.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
^ permalink raw reply
* Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read
From: David Miller @ 2018-04-30 15:38 UTC (permalink / raw)
To: soheil.kdev; +Cc: netdev, ycheng, ncardwell, edumazet, willemb, soheil
In-Reply-To: <20180427185733.36855-1-soheil.kdev@gmail.com>
From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com>
Date: Fri, 27 Apr 2018 14:57:32 -0400
> Since the socket lock is not held when calculating the size of
> receive queue, TCP_INQ is a hint. For example, it can overestimate
> the queue size by one byte, if FIN is received.
I think it is even worse than that.
If another application comes in and does a recvmsg() in parallel with
these calculations, you could even report a negative value.
These READ_ONCE() make it look like some of these issues are being
addressed but they are not.
You could freeze the values just by taking sk->sk_lock.slock, but I
don't know if that cost is considered acceptable or not.
Another idea is to sample both values in a loop, similar to a sequence
lock sequence:
again:
tmp1 = A;
tmp2 = B;
barrier();
tmp3 = A;
if (tmp1 != tmp3)
goto again;
But the current state of affairs is not going to work well.
^ permalink raw reply
* Re: simplify procfs code for seq_file instances V2
From: David Howells @ 2018-04-30 15:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: dhowells, Andrew Morton, Alexander Viro, linux-rtc,
Alessandro Zummo, Alexandre Belloni, devel, linux-kernel,
linux-scsi, linux-ide, Greg Kroah-Hartman, jfs-discussion,
linux-afs, linux-acpi, netdev, netfilter-devel, Jiri Slaby,
linux-ext4, Alexey Dobriyan, megaraidlinux.pdl, drbd-dev
In-Reply-To: <20180425154827.32251-1-hch@lst.de>
Note that your kernel hits the:
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes:
(ptrval) (fs_reclaim){?.+.}, at: fs_reclaim_acquire+0x12/0x35
{HARDIRQ-ON-W} state was registered at:
fs_reclaim_acquire+0x32/0x35
kmem_cache_alloc_node_trace+0x49/0x2cf
alloc_worker+0x1d/0x49
init_rescuer.part.7+0x19/0x8f
workqueue_init+0xc0/0x1fe
kernel_init_freeable+0xdc/0x433
kernel_init+0xa/0xf5
ret_from_fork+0x24/0x30
bug, as described here:
https://groups.google.com/forum/#!msg/syzkaller-bugs/sJC3Y3hOM08/aO3z9JXoAgAJ
David
^ permalink raw reply
* Re: [PATCH RFC iproute2-next 2/2] rdma: print provider resource attributes
From: Stephen Hemminger @ 2018-04-30 15:25 UTC (permalink / raw)
To: Steve Wise; +Cc: dsahern, leon, netdev, linux-rdma
In-Reply-To: <9dc07757af0a98c444c5a40131a7c54922361e03.1525100473.git.swise@opengridcomputing.com>
On Mon, 30 Apr 2018 07:36:18 -0700
Steve Wise <swise@opengridcomputing.com> wrote:
> +#define nla_type(attr) ((attr)->nla_type & NLA_TYPE_MASK)
> +
> +void newline(struct rd *rd)
> +{
> + if (rd->json_output)
> + jsonw_end_array(rd->jw);
> + else
> + pr_out("\n");
> +}
> +
> +void newline_indent(struct rd *rd)
> +{
> + newline(rd);
> + if (!rd->json_output)
> + pr_out(" ");
> +}
> +
> +static int print_provider_string(struct rd *rd, const char *key_str,
> + const char *val_str)
> +{
> + if (rd->json_output) {
> + jsonw_string_field(rd->jw, key_str, val_str);
> + return 0;
> + } else {
> + return pr_out("%s %s ", key_str, val_str);
> + }
> +}
> +
> +static int print_provider_s32(struct rd *rd, const char *key_str, int32_t val,
> + enum rdma_nldev_print_type print_type)
> +{
> + if (rd->json_output) {
> + jsonw_int_field(rd->jw, key_str, val);
> + return 0;
> + }
> + switch (print_type) {
> + case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
> + return pr_out("%s %d ", key_str, val);
> + case RDMA_NLDEV_PRINT_TYPE_HEX:
> + return pr_out("%s 0x%x ", key_str, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
This code should get converted to json_print library that handles the
different output modes; rather than rolling it's own equivalent functionality.
^ permalink raw reply
* Re: [PATCH] connector: add parent pid and tgid to coredump and exit events
From: David Miller @ 2018-04-30 15:17 UTC (permalink / raw)
To: zbr
Cc: stefan.strogin, jderehag, netdev, linux-kernel, xe-linux-external,
matt.helsley
In-Reply-To: <4667631525100490@web38o.yandex.ru>
From: Evgeniy Polyakov <zbr@ioremap.net>
Date: Mon, 30 Apr 2018 18:01:30 +0300
> Stefan, hi
>
> Sorry for delay.
>
> 26.04.2018, 15:04, "Stefan Strogin" <stefan.strogin@gmail.com>:
>> Hi David, Evgeniy,
>>
>> Sorry to bother you, but could you please comment about the UAPI change and the patch?
>
> With 4-bytes pid_t everything looks fine, and I do not know arch where pid is larger currently, so it looks safe.
>
> David, please pull it into your tree, or should it go via different path?
>
> Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
After this much time it needs to be resubmitted.
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc
From: Quentin Monnet @ 2018-04-30 15:16 UTC (permalink / raw)
To: David Ahern, Alexei Starovoitov
Cc: daniel, ast, netdev, oss-drivers, Yonghong Song
In-Reply-To: <5cc7a2d9-2a76-c323-e607-2b0400758297@gmail.com>
2018-04-30 09:12 UTC-0600 ~ David Ahern <dsahern@gmail.com>
> On 4/30/18 9:08 AM, Alexei Starovoitov wrote:
>>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>>> index 530ff6588d8f..8daef7326bb7 100644
>>> --- a/include/uapi/linux/bpf.h
>>> +++ b/include/uapi/linux/bpf.h
>>> @@ -1770,33 +1770,33 @@ union bpf_attr {
>>> *
>>> * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>>> * Description
>>> - * Return a user or a kernel stack in bpf program provided buffer.
>>> - * To achieve this, the helper needs *ctx*, which is a pointer
>>> + * Return a user or a kernel stack in bpf program provided buffer.
>>> + * To achieve this, the helper needs *ctx*, which is a pointer
>> I still don't quite get the difference.
>> It's replacing 2 tabs in above with 1 space + 2 tabs ?
Yes, exactly (Plus in this case, the "::" a few line below has a missing
tab).
>> Can you please teach the python script to accept both?
>> I bet that will be recurring mistake and it's impossible to spot in code review.
> And checkpatch throws an error on the 1 space + 2 tabs so it gets
> confusing on which format should be used.
Sorry about that :/. I will send a patch to make the script more flexible.
Quentin
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] bpf: fix formatting for bpf_get_stack() helper doc
From: David Ahern @ 2018-04-30 15:12 UTC (permalink / raw)
To: Alexei Starovoitov, Quentin Monnet
Cc: daniel, ast, netdev, oss-drivers, Yonghong Song
In-Reply-To: <20180430150833.gt2di56f4jembb2f@ast-mbp.dhcp.thefacebook.com>
On 4/30/18 9:08 AM, Alexei Starovoitov wrote:
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 530ff6588d8f..8daef7326bb7 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1770,33 +1770,33 @@ union bpf_attr {
>> *
>> * int bpf_get_stack(struct pt_regs *regs, void *buf, u32 size, u64 flags)
>> * Description
>> - * Return a user or a kernel stack in bpf program provided buffer.
>> - * To achieve this, the helper needs *ctx*, which is a pointer
>> + * Return a user or a kernel stack in bpf program provided buffer.
>> + * To achieve this, the helper needs *ctx*, which is a pointer
>
> I still don't quite get the difference.
> It's replacing 2 tabs in above with 1 space + 2 tabs ?
> Can you please teach the python script to accept both?
> I bet that will be recurring mistake and it's impossible to spot in code review.
>
And checkpatch throws an error on the 1 space + 2 tabs so it gets
confusing on which format should be used.
^ 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