Netdev List
 help / color / mirror / Atom feed
* Re: [Linux-kernel-mentees][PATCH v2] packet: Fix undefined behavior in bit shift
From: Alexei Starovoitov @ 2019-06-27 17:05 UTC (permalink / raw)
  To: Shuah Khan
  Cc: David Miller, c0d1n61at3, linux-kernel-mentees,
	Network Development, LKML
In-Reply-To: <9687ddc6-3bdb-5b2a-2934-ed9c6921551d@linuxfoundation.org>

On Thu, Jun 27, 2019 at 9:54 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/27/19 10:22 AM, David Miller wrote:
> > From: Shuah Khan <skhan@linuxfoundation.org>
> > Date: Wed, 26 Jun 2019 21:32:52 -0600
> >
> >> On 6/26/19 9:25 PM, Jiunn Chang wrote:
> >>> Shifting signed 32-bit value by 31 bits is undefined.  Changing most
> >>> significant bit to unsigned.
> >>> Changes included in v2:
> >>>     - use subsystem specific subject lines
> >>>     - CC required mailing lists
> >>>
> >>
> >> These version change lines don't belong in the change log.
> >
> > For networking changes I actually like the change lines to be in the
> > commit log.  So please don't stray people this way, thanks.
> >
>
> As a general rule, please don't include change lines in the commit log.
> For networking changes that get sent to David and netdev, as David
> points out here, he likes them in the commit log, please include them
> in the commit log.
>
> I am working on FAQ (Frequently Answered Questions) section for mentees.
> I will add this to it.

Same for bpf trees.
We prefer developers put as much as info as possible into commit logs
and cover letters.
Explanation of v1->v2->v3 differences is invaluable not only at
the point of code review, but in the future.

^ permalink raw reply

* Re: [Linux-kernel-mentees][PATCH v2] packet: Fix undefined behavior in bit shift
From: Shuah Khan @ 2019-06-27 17:08 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David Miller, c0d1n61at3, linux-kernel-mentees,
	Network Development, LKML, Shuah Khan
In-Reply-To: <CAADnVQLxrwkgHY6sg98NVfAsG3EYeJLxAevskOUdB=gNQugfSg@mail.gmail.com>

On 6/27/19 11:05 AM, Alexei Starovoitov wrote:
> On Thu, Jun 27, 2019 at 9:54 AM Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 6/27/19 10:22 AM, David Miller wrote:
>>> From: Shuah Khan <skhan@linuxfoundation.org>
>>> Date: Wed, 26 Jun 2019 21:32:52 -0600
>>>
>>>> On 6/26/19 9:25 PM, Jiunn Chang wrote:
>>>>> Shifting signed 32-bit value by 31 bits is undefined.  Changing most
>>>>> significant bit to unsigned.
>>>>> Changes included in v2:
>>>>>      - use subsystem specific subject lines
>>>>>      - CC required mailing lists
>>>>>
>>>>
>>>> These version change lines don't belong in the change log.
>>>
>>> For networking changes I actually like the change lines to be in the
>>> commit log.  So please don't stray people this way, thanks.
>>>
>>
>> As a general rule, please don't include change lines in the commit log.
>> For networking changes that get sent to David and netdev, as David
>> points out here, he likes them in the commit log, please include them
>> in the commit log.
>>
>> I am working on FAQ (Frequently Answered Questions) section for mentees.
>> I will add this to it.
> 
> Same for bpf trees.
> We prefer developers put as much as info as possible into commit logs
> and cover letters.
> Explanation of v1->v2->v3 differences is invaluable not only at
> the point of code review, but in the future.
> 

Thanks Alex. I will add that to the FAQ.

-- Shuah

^ permalink raw reply

* [PATCH net] net/ibmvnic: Report last valid speed and duplex values to ethtool
From: Thomas Falcon @ 2019-06-27 17:09 UTC (permalink / raw)
  To: netdev; +Cc: linuxppc-dev, bjking1, pradeep, dnbanerg, Thomas Falcon

This patch resolves an issue with sensitive bonding modes
that require valid speed and duplex settings to function
properly. Currently, the adapter will report that device
speed and duplex is unknown if the communication link
with firmware is unavailable. This decision can break LACP
configurations if the timing is right.

For example, if invalid speeds are reported, the slave
device's link state is set to a transitional "fail" state
and the LACP port is disabled. However, if valid speeds
are reported later but the link state has not been altered,
the LACP port will remain disabled. If the link state then
transitions back to "up" from "fail," it results in a state
such that the slave reports valid speed/duplex and is up,
but the LACP port will remain disabled.

Workaround this by reporting the last recorded speed
and duplex settings unless the device has never been
activated. In that case or when the hypervisor gives
invalid values, continue to report unknown speed or
duplex to ethtool.

Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 3da6800..7c14e33 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2276,10 +2276,8 @@ static int ibmvnic_get_link_ksettings(struct net_device *netdev,
 	int rc;
 
 	rc = send_query_phys_parms(adapter);
-	if (rc) {
-		adapter->speed = SPEED_UNKNOWN;
-		adapter->duplex = DUPLEX_UNKNOWN;
-	}
+	if (rc)
+		netdev_warn(netdev, "Device query of current speed and duplex settings failed; reported values may be stale.\n");
 	cmd->base.speed = adapter->speed;
 	cmd->base.duplex = adapter->duplex;
 	cmd->base.port = PORT_FIBRE;
@@ -4834,6 +4832,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	dev_set_drvdata(&dev->dev, netdev);
 	adapter->vdev = dev;
 	adapter->netdev = netdev;
+	adapter->speed = SPEED_UNKNOWN;
+	adapter->duplex = DUPLEX_UNKNOWN;
 
 	ether_addr_copy(adapter->mac_addr, mac_addr_p);
 	ether_addr_copy(netdev->dev_addr, adapter->mac_addr);
-- 
1.8.3.1


^ permalink raw reply related

* Re: KMSAN: uninit-value in aesti_encrypt
From: syzbot @ 2019-06-27 17:14 UTC (permalink / raw)
  To: aviadye, borisp, daniel, davejwatson, davem, ebiggers, glider,
	herbert, john.fastabend, linux-crypto, linux-kernel, netdev,
	syzkaller-bugs
In-Reply-To: <000000000000a97a15058c50c52e@google.com>

syzbot has found a reproducer for the following crash on:

HEAD commit:    41550654 [UPSTREAM] KVM: x86: degrade WARN to pr_warn_rate..
git tree:       kmsan
console output: https://syzkaller.appspot.com/x/log.txt?x=11302ccba00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=40511ad0c5945201
dashboard link: https://syzkaller.appspot.com/bug?extid=6f50c99e8f6194bf363f
compiler:       clang version 9.0.0 (/home/glider/llvm/clang  
80fee25776c2fb61e74c1ecb1a523375c2500b69)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12906f79a00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14355961a00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6f50c99e8f6194bf363f@syzkaller.appspotmail.com

==================================================================
BUG: KMSAN: uninit-value in subshift crypto/aes_ti.c:148 [inline]
BUG: KMSAN: uninit-value in aesti_encrypt+0x1238/0x1bc0 crypto/aes_ti.c:292
CPU: 0 PID: 11119 Comm: syz-executor333 Not tainted 5.2.0-rc4+ #7
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+0x191/0x1f0 lib/dump_stack.c:113
  kmsan_report+0x162/0x2d0 mm/kmsan/kmsan.c:611
  __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:304
  subshift crypto/aes_ti.c:148 [inline]
  aesti_encrypt+0x1238/0x1bc0 crypto/aes_ti.c:292
  crypto_cipher_encrypt_one include/linux/crypto.h:1753 [inline]
  crypto_cbcmac_digest_update+0x3cf/0x550 crypto/ccm.c:871
  crypto_shash_update crypto/shash.c:107 [inline]
  shash_ahash_finup+0x659/0xb20 crypto/shash.c:276
  shash_async_finup+0xbb/0x110 crypto/shash.c:291
  crypto_ahash_op+0x1cd/0x6e0 crypto/ahash.c:368
  crypto_ahash_finup+0x8c/0xb0 crypto/ahash.c:393
  crypto_ccm_auth+0x14b2/0x1570 crypto/ccm.c:230
  crypto_ccm_encrypt+0x272/0x8d0 crypto/ccm.c:309
  crypto_aead_encrypt include/crypto/aead.h:331 [inline]
  tls_do_encryption net/tls/tls_sw.c:521 [inline]
  tls_push_record+0x341a/0x4f70 net/tls/tls_sw.c:730
  bpf_exec_tx_verdict+0x1454/0x1c90 net/tls/tls_sw.c:770
  tls_sw_sendmsg+0x15bd/0x2740 net/tls/tls_sw.c:1033
  inet_sendmsg+0x48e/0x750 net/ipv4/af_inet.c:798
  sock_sendmsg_nosec net/socket.c:646 [inline]
  sock_sendmsg net/socket.c:665 [inline]
  __sys_sendto+0x905/0xb90 net/socket.c:1958
  __do_sys_sendto net/socket.c:1970 [inline]
  __se_sys_sendto+0x107/0x130 net/socket.c:1966
  __x64_sys_sendto+0x6e/0x90 net/socket.c:1966
  do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:302
  entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x4402d9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffcef4112e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004402d9
RDX: ffffffffffffff7f RSI: 00000000200005c0 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 0000000000000000 R09: fffffffffffffd56
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401b60
R13: 0000000000401bf0 R14: 0000000000000000 R15: 0000000000000000

Uninit was stored to memory at:
  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:201 [inline]
  kmsan_save_stack mm/kmsan/kmsan.c:213 [inline]
  kmsan_internal_chain_origin+0xcc/0x150 mm/kmsan/kmsan.c:414
  __msan_chain_origin+0x6b/0xe0 mm/kmsan/kmsan_instr.c:200
  __crypto_xor+0x1e8/0x1470 crypto/algapi.c:1019
  crypto_xor include/crypto/algapi.h:214 [inline]
  crypto_cbcmac_digest_update+0x2ba/0x550 crypto/ccm.c:865
  crypto_shash_update crypto/shash.c:107 [inline]
  shash_ahash_finup+0x659/0xb20 crypto/shash.c:276
  shash_async_finup+0xbb/0x110 crypto/shash.c:291
  crypto_ahash_op+0x1cd/0x6e0 crypto/ahash.c:368
  crypto_ahash_finup+0x8c/0xb0 crypto/ahash.c:393
  crypto_ccm_auth+0x14b2/0x1570 crypto/ccm.c:230
  crypto_ccm_encrypt+0x272/0x8d0 crypto/ccm.c:309
  crypto_aead_encrypt include/crypto/aead.h:331 [inline]
  tls_do_encryption net/tls/tls_sw.c:521 [inline]
  tls_push_record+0x341a/0x4f70 net/tls/tls_sw.c:730
  bpf_exec_tx_verdict+0x1454/0x1c90 net/tls/tls_sw.c:770
  tls_sw_sendmsg+0x15bd/0x2740 net/tls/tls_sw.c:1033
  inet_sendmsg+0x48e/0x750 net/ipv4/af_inet.c:798
  sock_sendmsg_nosec net/socket.c:646 [inline]
  sock_sendmsg net/socket.c:665 [inline]
  __sys_sendto+0x905/0xb90 net/socket.c:1958
  __do_sys_sendto net/socket.c:1970 [inline]
  __se_sys_sendto+0x107/0x130 net/socket.c:1966
  __x64_sys_sendto+0x6e/0x90 net/socket.c:1966
  do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:302
  entry_SYSCALL_64_after_hwframe+0x63/0xe7

Uninit was created at:
  kmsan_save_stack_with_flags+0x37/0x70 mm/kmsan/kmsan.c:201
  kmsan_internal_alloc_meta_for_pages+0x123/0x510 mm/kmsan/kmsan_hooks.c:103
  kmsan_alloc_page+0x7a/0xf0 mm/kmsan/kmsan_hooks.c:247
  __alloc_pages_nodemask+0x144d/0x6020 mm/page_alloc.c:4700
  alloc_pages_current+0x6a0/0x9b0 mm/mempolicy.c:2132
  alloc_pages include/linux/gfp.h:511 [inline]
  skb_page_frag_refill+0x15e/0x560 net/core/sock.c:2349
  sk_page_frag_refill+0xa4/0x330 net/core/sock.c:2369
  sk_msg_alloc+0x203/0x1050 net/core/skmsg.c:37
  tls_alloc_encrypted_msg net/tls/tls_sw.c:284 [inline]
  tls_sw_sendmsg+0xb6a/0x2740 net/tls/tls_sw.c:953
  inet_sendmsg+0x48e/0x750 net/ipv4/af_inet.c:798
  sock_sendmsg_nosec net/socket.c:646 [inline]
  sock_sendmsg net/socket.c:665 [inline]
  __sys_sendto+0x905/0xb90 net/socket.c:1958
  __do_sys_sendto net/socket.c:1970 [inline]
  __se_sys_sendto+0x107/0x130 net/socket.c:1966
  __x64_sys_sendto+0x6e/0x90 net/socket.c:1966
  do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:302
  entry_SYSCALL_64_after_hwframe+0x63/0xe7
==================================================================


^ permalink raw reply

* Re: [RFC] longer netdev names proposal
From: David Ahern @ 2019-06-27 17:14 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: davem, jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <20190627094327.GF2424@nanopsycho>

On 6/27/19 3:43 AM, Jiri Pirko wrote:
> Hi all.
> 
> In the past, there was repeatedly discussed the IFNAMSIZ (16) limit for
> netdevice name length. Now when we have PF and VF representors
> with port names like "pfXvfY", it became quite common to hit this limit:
> 0123456789012345
> enp131s0f1npf0vf6
> enp131s0f1npf0vf22

QinQ (stacked vlans) is another example.

> 
> Since IFLA_NAME is just a string, I though it might be possible to use
> it to carry longer names as it is. However, the userspace tools, like
> iproute2, are doing checks before print out. So for example in output of
> "ip addr" when IFLA_NAME is longer than IFNAMSIZE, the netdevice is
> completely avoided.
> 
> So here is a proposal that might work:
> 1) Add a new attribute IFLA_NAME_EXT that could carry names longer than
>    IFNAMSIZE, say 64 bytes. The max size should be only defined in kernel,
>    user should be prepared for any string size.
> 2) Add a file in sysfs that would indicate that NAME_EXT is supported by
>    the kernel.

no sysfs files.

Johannes added infrastructure to retrieve the policy. That is a more
flexible and robust option for determining what the kernel supports.


> 3) Udev is going to look for the sysfs indication file. In case when
>    kernel supports long names, it will do rename to longer name, setting
>    IFLA_NAME_EXT. If not, it does what it does now - fail.
> 4) There are two cases that can happen during rename:
>    A) The name is shorter than IFNAMSIZ
>       -> both IFLA_NAME and IFLA_NAME_EXT would contain the same string:
>          original IFLA_NAME     = eth0
>          original IFLA_NAME_EXT = eth0
>          renamed  IFLA_NAME     = enp5s0f1npf0vf1
>          renamed  IFLA_NAME_EXT = enp5s0f1npf0vf1
>    B) The name is longer tha IFNAMSIZ
>       -> IFLA_NAME would contain the original one, IFLA_NAME_EXT would 
>          contain the new one:
>          original IFLA_NAME     = eth0
>          original IFLA_NAME_EXT = eth0
>          renamed  IFLA_NAME     = eth0
>          renamed  IFLA_NAME_EXT = enp131s0f1npf0vf22

so kernel side there will be 2 names for the same net_device?

> 
> This would allow the old tools to work with "eth0" and the new
> tools would work with "enp131s0f1npf0vf22". In sysfs, there would
> be symlink from one name to another.

I would prefer a solution that does not rely on sysfs hooks.

>       
> Also, there might be a warning added to kernel if someone works
> with IFLA_NAME that the userspace tool should be upgraded.

that seems like spam and confusion for the first few years of a new api.

> 
> Eventually, only IFLA_NAME_EXT is going to be used by everyone.
> 
> I'm aware there are other places where similar new attribute
> would have to be introduced too (ip rule for example).
> I'm not saying this is a simple work.
> 
> Question is what to do with the ioctl api (get ifindex etc). I would
> probably leave it as is and push tools to use rtnetlink instead.

The ioctl API is going to be a limiter here. ifconfig is still quite
prevalent and net-snmp still uses ioctl (as just 2 common examples).
snmp showing one set of names and rtnetlink s/w showing another is going
to be really confusing.

^ permalink raw reply

* Re: [PATCH net-next 00/12] s390/qeth: updates 2019-06-27
From: David Miller @ 2019-06-27 17:18 UTC (permalink / raw)
  To: jwi; +Cc: netdev, linux-s390, heiko.carstens, raspl, ubraun
In-Reply-To: <20190627150133.58746-1-jwi@linux.ibm.com>

From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Thu, 27 Jun 2019 17:01:21 +0200

> please apply another round of qeth updates for net-next.
> This completes the conversion of the control path to use dynamically
> allocated cmd buffers, along with some fine-tuning for the route
> validation fix that recently went into -net.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH bpf-next 0/3] capture integers in BTF type info for map defs
From: Song Liu @ 2019-06-27 17:22 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Alexei Starovoitov, daniel@iogearbox.net,
	Kernel Team, bpf@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190626232133.3800637-1-andriin@fb.com>



> On Jun 26, 2019, at 4:21 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> 
> This patch set implements an update to how BTF-defined maps are specified. The
> change is in how integer attributes, e.g., type, max_entries, map_flags, are
> specified: now they are captured as part of map definition struct's BTF type
> information (using array dimension), eliminating the need for compile-time
> data initialization and keeping all the metadata in one place.

Using array dimension is hacky. But I guess this work.



> 
> All existing selftests that were using BTF-defined maps are updated, along
> with some other selftests, that were switched to new syntax.
> 
> Andrii Nakryiko (3):
>  libbpf: capture value in BTF type info for BTF-defined map defs
>  selftests/bpf: convert selftests using BTF-defined maps to new syntax
>  selftests/bpf: convert legacy BPF maps to BTF-defined ones
> 
> tools/lib/bpf/libbpf.c                        |  58 +++++----
> tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
> tools/testing/selftests/bpf/progs/bpf_flow.c  |  28 ++---
> .../selftests/bpf/progs/get_cgroup_id_kern.c  |  26 ++---
> .../testing/selftests/bpf/progs/netcnt_prog.c |  20 ++--
> tools/testing/selftests/bpf/progs/pyperf.h    |  90 +++++++-------
> .../selftests/bpf/progs/sample_map_ret0.c     |  24 ++--
> .../selftests/bpf/progs/socket_cookie_prog.c  |  13 +--
> .../bpf/progs/sockmap_verdict_prog.c          |  48 ++++----
> .../testing/selftests/bpf/progs/strobemeta.h  |  68 +++++------
> .../selftests/bpf/progs/test_btf_newkv.c      |  13 +--
> .../bpf/progs/test_get_stack_rawtp.c          |  39 +++----
> .../selftests/bpf/progs/test_global_data.c    |  37 +++---
> tools/testing/selftests/bpf/progs/test_l4lb.c |  65 ++++-------
> .../selftests/bpf/progs/test_l4lb_noinline.c  |  65 ++++-------
> .../selftests/bpf/progs/test_map_in_map.c     |  30 ++---
> .../selftests/bpf/progs/test_map_lock.c       |  26 ++---
> .../testing/selftests/bpf/progs/test_obj_id.c |  12 +-
> .../bpf/progs/test_select_reuseport_kern.c    |  67 ++++-------
> .../bpf/progs/test_send_signal_kern.c         |  26 ++---
> .../bpf/progs/test_sock_fields_kern.c         |  78 +++++--------
> .../selftests/bpf/progs/test_spin_lock.c      |  36 +++---
> .../bpf/progs/test_stacktrace_build_id.c      |  55 ++++-----
> .../selftests/bpf/progs/test_stacktrace_map.c |  52 +++------
> .../selftests/bpf/progs/test_tcp_estats.c     |  13 +--
> .../selftests/bpf/progs/test_tcpbpf_kern.c    |  26 ++---
> .../selftests/bpf/progs/test_tcpnotify_kern.c |  28 ++---
> tools/testing/selftests/bpf/progs/test_xdp.c  |  26 ++---
> .../selftests/bpf/progs/test_xdp_loop.c       |  26 ++---
> .../selftests/bpf/progs/test_xdp_noinline.c   |  81 +++++--------
> .../selftests/bpf/progs/xdp_redirect_map.c    |  12 +-
> .../testing/selftests/bpf/progs/xdping_kern.c |  12 +-
> .../selftests/bpf/test_queue_stack_map.h      |  30 ++---
> .../testing/selftests/bpf/test_sockmap_kern.h | 110 +++++++++---------
> 34 files changed, 571 insertions(+), 772 deletions(-)
> 
> -- 
> 2.17.1
> 


^ permalink raw reply

* Re: [PATCH v4 00/13] net: Add generic and Allwinner YAML bindings
From: David Miller @ 2019-06-27 17:22 UTC (permalink / raw)
  To: maxime.ripard
  Cc: mark.rutland, robh+dt, frowand.list, wens, mcoquelin.stm32,
	alexandre.torgue, netdev, linux-arm-kernel, devicetree,
	linux-stm32, maxime.chevallier, antoine.tenart, andrew,
	f.fainelli, hkallweit1
In-Reply-To: <cover.e80da8845680a45c2e07d5f17280fdba84555b8a.1561649505.git-series.maxime.ripard@bootlin.com>

From: Maxime Ripard <maxime.ripard@bootlin.com>
Date: Thu, 27 Jun 2019 17:31:42 +0200

> This is an attempt at getting the main generic DT bindings for the ethernet
> (and related) devices, and convert some DT bindings for the Allwinner DTs
> to YAML as well.
> 
> This should provide some DT validation coverage.

I don't think this should go via my tree as it's all DT stuff.

^ permalink raw reply

* Re: [PATCH bpf-next 1/3] libbpf: capture value in BTF type info for BTF-defined map defs
From: Song Liu @ 2019-06-27 17:27 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Alexei Starovoitov, daniel@iogearbox.net,
	Kernel Team, bpf@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <20190626232133.3800637-2-andriin@fb.com>



> On Jun 26, 2019, at 4:21 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> 
> Change BTF-defined map definitions to capture compile-time integer
> values as part of BTF type definition, to avoid split of key/value type
> information and actual type/size/flags initialization for maps.

If I have an old bpf program and compiled it with new llvm, will it  
work with new libbpf? 


> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c                    | 58 +++++++++++------------
> tools/testing/selftests/bpf/bpf_helpers.h |  3 ++
> 2 files changed, 31 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 68f45a96769f..f2b02032a8e6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1028,40 +1028,40 @@ static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
> 	}
> }
> 
> -static bool get_map_field_int(const char *map_name,
> -			      const struct btf *btf,
> +/*
> + * Fetch integer attribute of BTF map definition. Such attributes are
> + * represented using a pointer to an array, in which dimensionality of array
> + * encodes specified integer value. E.g., int (*type)[BPF_MAP_TYPE_ARRAY];
> + * encodes `type => BPF_MAP_TYPE_ARRAY` key/value pair completely using BTF
> + * type definition, while using only sizeof(void *) space in ELF data section.
> + */
> +static bool get_map_field_int(const char *map_name, const struct btf *btf,
> 			      const struct btf_type *def,
> -			      const struct btf_member *m,
> -			      const void *data, __u32 *res) {
> +			      const struct btf_member *m, __u32 *res) {
> 	const struct btf_type *t = skip_mods_and_typedefs(btf, m->type);
> 	const char *name = btf__name_by_offset(btf, m->name_off);
> -	__u32 int_info = *(const __u32 *)(const void *)(t + 1);
> +	const struct btf_array *arr_info;
> +	const struct btf_type *arr_t;
> 
> -	if (BTF_INFO_KIND(t->info) != BTF_KIND_INT) {
> -		pr_warning("map '%s': attr '%s': expected INT, got %u.\n",
> +	if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR) {
> +		pr_warning("map '%s': attr '%s': expected PTR, got %u.\n",
> 			   map_name, name, BTF_INFO_KIND(t->info));
> 		return false;
> 	}
> -	if (t->size != 4 || BTF_INT_BITS(int_info) != 32 ||
> -	    BTF_INT_OFFSET(int_info)) {
> -		pr_warning("map '%s': attr '%s': expected 32-bit non-bitfield integer, "
> -			   "got %u-byte (%d-bit) one with bit offset %d.\n",
> -			   map_name, name, t->size, BTF_INT_BITS(int_info),
> -			   BTF_INT_OFFSET(int_info));
> -		return false;
> -	}
> -	if (BTF_INFO_KFLAG(def->info) && BTF_MEMBER_BITFIELD_SIZE(m->offset)) {
> -		pr_warning("map '%s': attr '%s': bitfield is not supported.\n",
> -			   map_name, name);
> +
> +	arr_t = btf__type_by_id(btf, t->type);
> +	if (!arr_t) {
> +		pr_warning("map '%s': attr '%s': type [%u] not found.\n",
> +			   map_name, name, t->type);
> 		return false;
> 	}
> -	if (m->offset % 32) {
> -		pr_warning("map '%s': attr '%s': unaligned fields are not supported.\n",
> -			   map_name, name);
> +	if (BTF_INFO_KIND(arr_t->info) != BTF_KIND_ARRAY) {
> +		pr_warning("map '%s': attr '%s': expected ARRAY, got %u.\n",
> +			   map_name, name, BTF_INFO_KIND(arr_t->info));
> 		return false;
> 	}
> -
> -	*res = *(const __u32 *)(data + m->offset / 8);
> +	arr_info = (const void *)(arr_t + 1);
> +	*res = arr_info->nelems;
> 	return true;
> }
> 
> @@ -1074,7 +1074,6 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> 	const struct btf_var_secinfo *vi;
> 	const struct btf_var *var_extra;
> 	const struct btf_member *m;
> -	const void *def_data;
> 	const char *map_name;
> 	struct bpf_map *map;
> 	int vlen, i;
> @@ -1131,7 +1130,6 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> 	pr_debug("map '%s': at sec_idx %d, offset %zu.\n",
> 		 map_name, map->sec_idx, map->sec_offset);
> 
> -	def_data = data->d_buf + vi->offset;
> 	vlen = BTF_INFO_VLEN(def->info);
> 	m = (const void *)(def + 1);
> 	for (i = 0; i < vlen; i++, m++) {
> @@ -1144,19 +1142,19 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> 		}
> 		if (strcmp(name, "type") == 0) {
> 			if (!get_map_field_int(map_name, obj->btf, def, m,
> -					       def_data, &map->def.type))
> +					       &map->def.type))
> 				return -EINVAL;
> 			pr_debug("map '%s': found type = %u.\n",
> 				 map_name, map->def.type);
> 		} else if (strcmp(name, "max_entries") == 0) {
> 			if (!get_map_field_int(map_name, obj->btf, def, m,
> -					       def_data, &map->def.max_entries))
> +					       &map->def.max_entries))
> 				return -EINVAL;
> 			pr_debug("map '%s': found max_entries = %u.\n",
> 				 map_name, map->def.max_entries);
> 		} else if (strcmp(name, "map_flags") == 0) {
> 			if (!get_map_field_int(map_name, obj->btf, def, m,
> -					       def_data, &map->def.map_flags))
> +					       &map->def.map_flags))
> 				return -EINVAL;
> 			pr_debug("map '%s': found map_flags = %u.\n",
> 				 map_name, map->def.map_flags);
> @@ -1164,7 +1162,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> 			__u32 sz;
> 
> 			if (!get_map_field_int(map_name, obj->btf, def, m,
> -					       def_data, &sz))
> +					       &sz))
> 				return -EINVAL;
> 			pr_debug("map '%s': found key_size = %u.\n",
> 				 map_name, sz);
> @@ -1207,7 +1205,7 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
> 			__u32 sz;
> 
> 			if (!get_map_field_int(map_name, obj->btf, def, m,
> -					       def_data, &sz))
> +					       &sz))
> 				return -EINVAL;
> 			pr_debug("map '%s': found value_size = %u.\n",
> 				 map_name, sz);
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 1a5b1accf091..aa5ddf58c088 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -8,6 +8,9 @@
>  */
> #define SEC(NAME) __attribute__((section(NAME), used))
> 
> +#define __int(name, val) int (*name)[val]
> +#define __type(name, val) val *name
> +

I think we need these two in libbpf. 

Thanks,
Song

> /* helper macro to print out debug messages */
> #define bpf_printk(fmt, ...)				\
> ({							\
> -- 
> 2.17.1
> 


^ permalink raw reply

* Re: [Linux-kernel-mentees][PATCH v2] packet: Fix undefined behavior in bit shift
From: David Miller @ 2019-06-27 17:34 UTC (permalink / raw)
  To: c0d1n61at3; +Cc: skhan, linux-kernel-mentees, netdev, linux-kernel
In-Reply-To: <20190627165726.p6k3tugjs2gzgnjh@rYz3n>

From: Jiunn Chang <c0d1n61at3@gmail.com>
Date: Thu, 27 Jun 2019 11:57:28 -0500

> On Thu, Jun 27, 2019 at 09:22:53AM -0700, David Miller wrote:
>> From: Shuah Khan <skhan@linuxfoundation.org>
>> Date: Wed, 26 Jun 2019 21:32:52 -0600
>> 
>> > On 6/26/19 9:25 PM, Jiunn Chang wrote:
>> >> Shifting signed 32-bit value by 31 bits is undefined.  Changing most
>> >> significant bit to unsigned.
>> >> Changes included in v2:
>> >>    - use subsystem specific subject lines
>> >>    - CC required mailing lists
>> >> 
>> > 
>> > These version change lines don't belong in the change log.
>> 
>> For networking changes I actually like the change lines to be in the
>> commit log.  So please don't stray people this way, thanks.
> 
> Hello David,
> 
> Would you like me to send v3 with the change log in the patch description?

I'll use v2 which had this done correctly.

^ permalink raw reply

* [PATCH 08/87] atm: idt77252: Remove call to memset after dma_alloc_coherent
From: Fuqian Huang @ 2019-06-27 17:34 UTC (permalink / raw)
  Cc: Fuqian Huang, Chas Williams, linux-atm-general, netdev,
	linux-kernel

In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/atm/idt77252.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index 43a14579e80e..df51680e8931 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -1379,7 +1379,6 @@ init_tsq(struct idt77252_dev *card)
 		printk("%s: can't allocate TSQ.\n", card->name);
 		return -1;
 	}
-	memset(card->tsq.base, 0, TSQSIZE);
 
 	card->tsq.last = card->tsq.base + TSQ_NUM_ENTRIES - 1;
 	card->tsq.next = card->tsq.last;
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH net-next 00/16] mlxsw: PTP timestamping support
From: Ido Schimmel @ 2019-06-27 17:35 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, davem, jiri, petrm, mlxsw, Ido Schimmel
In-Reply-To: <20190627165134.zg7rdph2ct377bel@localhost>

On Thu, Jun 27, 2019 at 09:51:34AM -0700, Richard Cochran wrote:
> On Thu, Jun 27, 2019 at 04:52:43PM +0300, Ido Schimmel wrote:
> > From: Ido Schimmel <idosch@mellanox.com>
> > 
> > This is the second patchset adding PTP support in mlxsw. Next patchset
> > will add PTP shapers which are required to maintain accuracy under rates
> > lower than 40Gb/s, while subsequent patchsets will add tracepoints and
> > selftests.
> 
> Please add the PTP maintainer onto CC for PTP patch submissions.

No problem. To be clear, I didn't Cc you since this is all internal to
mlxsw.

I see David made a style comment. We can wait with v2 to your comments,
if you plan to review this patchset.

Thanks

^ permalink raw reply

* [PATCH 0/2] tls, add unhash callback
From: John Fastabend @ 2019-06-27 17:36 UTC (permalink / raw)
  To: daniel, jakub.kicinski, ast; +Cc: netdev, edumazet, john.fastabend, bpf

Resolve a series of splats discovered by syzbot and noted by
Eric Dumazet. The primary problem here is we resolved an issue on
the BPF sockmap side by adding an unhash callback. This is
required to ensure sockmap sockets do not transition out of
ESTABLISHED state into a LISTEN state. When we did this it
created a case where the interaction between callbacks in TLS
and sockmap when used together could break. This resulted in
leaking TLS memory and potential to build loops of callbacks
where sockmap called into TLS and TLS called back into BPF.

Additionally, TLS was releasing the sock lock and then
reaquiring it during the tear down process which could hang
if another sock operation happened while the lock was not
held.

To fix this first refactor TLS code so lock is held for the
entire teardown operation. Then add an unhash callback to ensure
TLS can not transition from ESTABLISHED to LISTEN state. This
transition is a similar bug to the one found and fixed previously
in sockmap. And cleans up the callbacks to fix the syzbot
errors.

---

John Fastabend (2):
      tls: remove close callback sock unlock/lock and flush_sync
      bpf: tls, implement unhash to avoid transition out of ESTABLISHED


 include/net/tls.h  |    6 ++-
 net/tls/tls_main.c |   96 ++++++++++++++++++++++++++++++++++++----------------
 net/tls/tls_sw.c   |   50 ++++++++++++++++++---------
 3 files changed, 103 insertions(+), 49 deletions(-)

--
Signature

^ permalink raw reply

* [PATCH 1/2] tls: remove close callback sock unlock/lock and flush_sync
From: John Fastabend @ 2019-06-27 17:36 UTC (permalink / raw)
  To: daniel, jakub.kicinski, ast; +Cc: netdev, edumazet, john.fastabend, bpf
In-Reply-To: <156165697019.32598.7171757081688035707.stgit@john-XPS-13-9370>

The tls close() callback currently drops the sock lock, makes a
cancel_delayed_work_sync() call, and then relocks the sock. This
seems suspect at best. The lock_sock() is applied to stop concurrent
operations on the socket while tearing the sock down. Further we
will need to add support for unhash() shortly and this complicates
matters because the lock may or may not be held then.

So to fix the above situation and simplify the next patch to add
unhash this patch creates a function tls_sk_proto_cleanup() that
tears down the socket without calling lock_sock/release_sock. In
order to flush the workqueue then we do the following,

  - Add a new bit to ctx, BIT_TX_CLOSING that is set when the
    tls resources are being removed.
  - Check this bit before scheduling any new work. This way we
    avoid queueing new work after tear down has started.
  - With the BIT_TX_CLOSING ensuring no new work is being added
    convert the cancel_delayed_work_sync to flush_delayed_work()
  - Finally call tlx_tx_records() to complete any available records
    before,
  - releasing and removing tls ctx.

The above is implemented for the software case namely any of
the following configurations from build_protos,

   prot[TLS_SW][TLS_BASE]
   prot[TLS_BASE][TLS_SW]
   prot[TLS_SW][TLS_SW]

The implication is a follow up patch is needed to resolve the
hardware offload case.

Tested with net selftests and bpf selftests.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 include/net/tls.h  |    4 ++--
 net/tls/tls_main.c |   54 ++++++++++++++++++++++++++--------------------------
 net/tls/tls_sw.c   |   50 ++++++++++++++++++++++++++++++++----------------
 3 files changed, 62 insertions(+), 46 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 4a55ce6a303f..6fe1f5c96f4a 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -105,9 +105,7 @@ struct tls_device {
 enum {
 	TLS_BASE,
 	TLS_SW,
-#ifdef CONFIG_TLS_DEVICE
 	TLS_HW,
-#endif
 	TLS_HW_RECORD,
 	TLS_NUM_CONFIG,
 };
@@ -160,6 +158,7 @@ struct tls_sw_context_tx {
 	int async_capable;
 
 #define BIT_TX_SCHEDULED	0
+#define BIT_TX_CLOSING		1
 	unsigned long tx_bitmask;
 };
 
@@ -327,6 +326,7 @@ void tls_sw_close(struct sock *sk, long timeout);
 void tls_sw_free_resources_tx(struct sock *sk);
 void tls_sw_free_resources_rx(struct sock *sk);
 void tls_sw_release_resources_rx(struct sock *sk);
+void tls_sw_release_strp_rx(struct tls_context *tls_ctx);
 int tls_sw_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		   int nonblock, int flags, int *addr_len);
 bool tls_sw_stream_read(const struct sock *sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index fc81ae18cc44..51cb19e24dd9 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -261,24 +261,9 @@ static void tls_ctx_free(struct tls_context *ctx)
 	kfree(ctx);
 }
 
-static void tls_sk_proto_close(struct sock *sk, long timeout)
+static void tls_sk_proto_cleanup(struct sock *sk,
+				 struct tls_context *ctx, long timeo)
 {
-	struct tls_context *ctx = tls_get_ctx(sk);
-	long timeo = sock_sndtimeo(sk, 0);
-	void (*sk_proto_close)(struct sock *sk, long timeout);
-	bool free_ctx = false;
-
-	lock_sock(sk);
-	sk_proto_close = ctx->sk_proto_close;
-
-	if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
-		goto skip_tx_cleanup;
-
-	if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
-		free_ctx = true;
-		goto skip_tx_cleanup;
-	}
-
 	if (!tls_complete_pending_work(sk, ctx, 0, &timeo))
 		tls_handle_open_record(sk, 0);
 
@@ -299,22 +284,37 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
 #ifdef CONFIG_TLS_DEVICE
 	if (ctx->rx_conf == TLS_HW)
 		tls_device_offload_cleanup_rx(sk);
-
-	if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW) {
-#else
-	{
 #endif
-		tls_ctx_free(ctx);
-		ctx = NULL;
+}
+
+static void tls_sk_proto_close(struct sock *sk, long timeout)
+{
+	struct tls_context *ctx = tls_get_ctx(sk);
+	long timeo = sock_sndtimeo(sk, 0);
+	void (*sk_proto_close)(struct sock *sk, long timeout);
+	bool free_ctx = false;
+
+	lock_sock(sk);
+	sk_proto_close = ctx->sk_proto_close;
+
+	if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
+		goto skip_tx_cleanup;
+
+	if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
+		free_ctx = true;
+		goto skip_tx_cleanup;
 	}
 
+	tls_sk_proto_cleanup(sk, ctx, timeo);
+
 skip_tx_cleanup:
 	release_sock(sk);
+	if (ctx->rx_conf == TLS_SW)
+		tls_sw_release_strp_rx(ctx);
 	sk_proto_close(sk, timeout);
-	/* free ctx for TLS_HW_RECORD, used by tcp_set_state
-	 * for sk->sk_prot->unhash [tls_hw_unhash]
-	 */
-	if (free_ctx)
+
+	if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW &&
+	    ctx->tx_conf != TLS_HW_RECORD && ctx->rx_conf != TLS_HW_RECORD)
 		tls_ctx_free(ctx);
 }
 
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 455a782c7658..d234a6b818e6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -473,7 +473,8 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err)
 		return;
 
 	/* Schedule the transmission */
-	if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
+	if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask) &&
+	    !test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask))
 		schedule_delayed_work(&ctx->tx_work.work, 1);
 }
 
@@ -2058,16 +2059,26 @@ void tls_sw_free_resources_tx(struct sock *sk)
 	struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
 	struct tls_rec *rec, *tmp;
 
+	/* Set TX CLOSING bit to stop tx_work from being scheduled
+	 * while tearing down TX context. We will flush any pending
+	 * work before free'ing ctx anyways. If already set then
+	 * another call is already free'ing resources.
+	 */
+	if (test_and_set_bit(BIT_TX_CLOSING, &ctx->tx_bitmask))
+		return;
+
 	/* Wait for any pending async encryptions to complete */
 	smp_store_mb(ctx->async_notify, true);
 	if (atomic_read(&ctx->encrypt_pending))
 		crypto_wait_req(-EINPROGRESS, &ctx->async_wait);
 
-	release_sock(sk);
-	cancel_delayed_work_sync(&ctx->tx_work.work);
-	lock_sock(sk);
-
-	/* Tx whatever records we can transmit and abandon the rest */
+	/* Flush work queue and then Tx whatever records we can
+	 * transmit and abandon the rest, lock_sock(sk) must be
+	 * held here. We ensure no further work is enqueue by
+	 * checking CLOSING bit before queueing new work and
+	 * setting it above.
+	 */
+	flush_delayed_work(&ctx->tx_work.work);
 	tls_tx_records(sk, -1);
 
 	/* Free up un-sent records in tx_list. First, free
@@ -2111,22 +2122,22 @@ void tls_sw_release_resources_rx(struct sock *sk)
 		write_lock_bh(&sk->sk_callback_lock);
 		sk->sk_data_ready = ctx->saved_data_ready;
 		write_unlock_bh(&sk->sk_callback_lock);
-		release_sock(sk);
-		strp_done(&ctx->strp);
-		lock_sock(sk);
 	}
 }
 
-void tls_sw_free_resources_rx(struct sock *sk)
+void tls_sw_release_strp_rx(struct tls_context *tls_ctx)
 {
-	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 
-	tls_sw_release_resources_rx(sk);
-
+	strp_done(&ctx->strp);
 	kfree(ctx);
 }
 
+void tls_sw_free_resources_rx(struct sock *sk)
+{
+	tls_sw_release_resources_rx(sk);
+}
+
 /* The work handler to transmitt the encrypted records in tx_list */
 static void tx_work_handler(struct work_struct *work)
 {
@@ -2140,9 +2151,14 @@ static void tx_work_handler(struct work_struct *work)
 	if (!test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
 		return;
 
-	lock_sock(sk);
+	/* If we are running from a socket close operation then the
+	 * lock is already held so we do not need to hold it.
+	 */
+	if (likely(!test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)))
+		lock_sock(sk);
 	tls_tx_records(sk, -1);
-	release_sock(sk);
+	if (likely(!test_bit(BIT_TX_CLOSING, &ctx->tx_bitmask)))
+		release_sock(sk);
 }
 
 void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
@@ -2152,8 +2168,8 @@ void tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
 	/* Schedule the transmission if tx list is ready */
 	if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
 		/* Schedule the transmission */
-		if (!test_and_set_bit(BIT_TX_SCHEDULED,
-				      &tx_ctx->tx_bitmask))
+		if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask) &&
+		    !test_bit(BIT_TX_CLOSING, &tx_ctx->tx_bitmask))
 			schedule_delayed_work(&tx_ctx->tx_work.work, 0);
 	}
 }


^ permalink raw reply related

* [PATCH 2/2] bpf: tls, implement unhash to avoid transition out of ESTABLISHED
From: John Fastabend @ 2019-06-27 17:36 UTC (permalink / raw)
  To: daniel, jakub.kicinski, ast; +Cc: netdev, edumazet, john.fastabend, bpf
In-Reply-To: <156165697019.32598.7171757081688035707.stgit@john-XPS-13-9370>

It is possible (via shutdown()) for TCP socks to go through TCP_CLOSE
state via tcp_disconnect() without calling into close callback. This
would allow a kTLS enabled socket to exist outside of ESTABLISHED
state which is not supported.

Solve this the same way we solved the sock{map|hash} case by adding
an unhash hook to remove tear down the TLS state.

Tested with bpf and net selftests plus ran syzkaller reproducers
for below listed issues.

Fixes: d91c3e17f75f2 ("net/tls: Only attach to sockets in ESTABLISHED state")
Reported-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot+4207c7f3a443366d8aa2@syzkaller.appspotmail.com
Reported-by: syzbot+06537213db7ba2745c4a@syzkaller.appspotmail.com
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 include/net/tls.h  |    2 ++
 net/tls/tls_main.c |   50 +++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 6fe1f5c96f4a..935d65606bb3 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -264,6 +264,8 @@ struct tls_context {
 	bool in_tcp_sendpages;
 	bool pending_open_record_frags;
 
+	struct proto *sk_proto;
+
 	int (*push_pending_record)(struct sock *sk, int flags);
 
 	void (*sk_write_space)(struct sock *sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 51cb19e24dd9..e1750634a53a 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -251,11 +251,16 @@ static void tls_write_space(struct sock *sk)
 	ctx->sk_write_space(sk);
 }
 
-static void tls_ctx_free(struct tls_context *ctx)
+static void tls_ctx_free(struct sock *sk, struct tls_context *ctx)
 {
+	struct inet_connection_sock *icsk = inet_csk(sk);
+
 	if (!ctx)
 		return;
 
+	sk->sk_prot = ctx->sk_proto;
+	icsk->icsk_ulp_data = NULL;
+
 	memzero_explicit(&ctx->crypto_send, sizeof(ctx->crypto_send));
 	memzero_explicit(&ctx->crypto_recv, sizeof(ctx->crypto_recv));
 	kfree(ctx);
@@ -287,23 +292,49 @@ static void tls_sk_proto_cleanup(struct sock *sk,
 #endif
 }
 
+static void tls_sk_proto_unhash(struct sock *sk)
+{
+	struct tls_context *ctx = tls_get_ctx(sk);
+	void (*sk_proto_unhash)(struct sock *sk);
+	long timeo = sock_sndtimeo(sk, 0);
+
+	if (unlikely(!ctx)) {
+		if (sk->sk_prot->unhash)
+			sk->sk_prot->unhash(sk);
+		return;
+	}
+
+	sk->sk_prot = ctx->sk_proto;
+	sk_proto_unhash = ctx->unhash;
+	tls_sk_proto_cleanup(sk, ctx, timeo);
+	if (ctx->rx_conf == TLS_SW)
+		tls_sw_release_strp_rx(ctx);
+	tls_ctx_free(sk, ctx);
+	if (sk_proto_unhash)
+		sk_proto_unhash(sk);
+}
+
 static void tls_sk_proto_close(struct sock *sk, long timeout)
 {
 	struct tls_context *ctx = tls_get_ctx(sk);
 	long timeo = sock_sndtimeo(sk, 0);
 	void (*sk_proto_close)(struct sock *sk, long timeout);
-	bool free_ctx = false;
+
+	if (unlikely(!ctx)) {
+		if (sk->sk_prot->close)
+			sk->sk_prot->close(sk, timeout);
+		return;
+	}
 
 	lock_sock(sk);
+	sk->sk_prot = ctx->sk_proto;
 	sk_proto_close = ctx->sk_proto_close;
 
 	if (ctx->tx_conf == TLS_HW_RECORD && ctx->rx_conf == TLS_HW_RECORD)
 		goto skip_tx_cleanup;
 
-	if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
-		free_ctx = true;
+	if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE)
 		goto skip_tx_cleanup;
-	}
 
 	tls_sk_proto_cleanup(sk, ctx, timeo);
 
@@ -311,11 +342,12 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
 	release_sock(sk);
 	if (ctx->rx_conf == TLS_SW)
 		tls_sw_release_strp_rx(ctx);
-	sk_proto_close(sk, timeout);
 
 	if (ctx->tx_conf != TLS_HW && ctx->rx_conf != TLS_HW &&
 	    ctx->tx_conf != TLS_HW_RECORD && ctx->rx_conf != TLS_HW_RECORD)
-		tls_ctx_free(ctx);
+		tls_ctx_free(sk, ctx);
+	if (sk_proto_close)
+		sk_proto_close(sk, timeout);
 }
 
 static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
@@ -733,16 +765,19 @@ static void build_protos(struct proto prot[TLS_NUM_CONFIG][TLS_NUM_CONFIG],
 	prot[TLS_SW][TLS_BASE] = prot[TLS_BASE][TLS_BASE];
 	prot[TLS_SW][TLS_BASE].sendmsg		= tls_sw_sendmsg;
 	prot[TLS_SW][TLS_BASE].sendpage		= tls_sw_sendpage;
+	prot[TLS_SW][TLS_BASE].unhash		= tls_sk_proto_unhash;
 
 	prot[TLS_BASE][TLS_SW] = prot[TLS_BASE][TLS_BASE];
 	prot[TLS_BASE][TLS_SW].recvmsg		  = tls_sw_recvmsg;
 	prot[TLS_BASE][TLS_SW].stream_memory_read = tls_sw_stream_read;
 	prot[TLS_BASE][TLS_SW].close		  = tls_sk_proto_close;
+	prot[TLS_BASE][TLS_SW].unhash		= tls_sk_proto_unhash;
 
 	prot[TLS_SW][TLS_SW] = prot[TLS_SW][TLS_BASE];
 	prot[TLS_SW][TLS_SW].recvmsg		= tls_sw_recvmsg;
 	prot[TLS_SW][TLS_SW].stream_memory_read	= tls_sw_stream_read;
 	prot[TLS_SW][TLS_SW].close		= tls_sk_proto_close;
+	prot[TLS_SW][TLS_SW].unhash		= tls_sk_proto_unhash;
 
 #ifdef CONFIG_TLS_DEVICE
 	prot[TLS_HW][TLS_BASE] = prot[TLS_BASE][TLS_BASE];
@@ -793,6 +828,7 @@ static int tls_init(struct sock *sk)
 	tls_build_proto(sk);
 	ctx->tx_conf = TLS_BASE;
 	ctx->rx_conf = TLS_BASE;
+	ctx->sk_proto = sk->sk_prot;
 	update_sk_prot(sk, ctx);
 out:
 	return rc;


^ permalink raw reply related

* Re: [PATCH net-next 2/3] net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK reports
From: Russell King - ARM Linux admin @ 2019-06-27 17:37 UTC (permalink / raw)
  To: Vladimir Oltean; +Cc: f.fainelli, vivien.didelot, andrew, davem, netdev
In-Reply-To: <20190626112014.7625-3-olteanv@gmail.com>

On Wed, Jun 26, 2019 at 02:20:13PM +0300, Vladimir Oltean wrote:
> PHYLINK being designed with PHYs in mind that can change MII protocol,
> for correct operation it is necessary to ensure that the PHY interface
> mode stays the same (otherwise clear the supported bit mask, as
> required).
> 
> Because this is just a hypothetical situation for now, we don't bother
> to check whether we could actually support the new PHY interface mode.
> Actually we could modify the xMII table, reset the switch and send an
> updated static configuration, but adding that would just be dead code.
> 
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> ---
>  drivers/net/dsa/sja1105/sja1105_main.c | 47 ++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
> index da1736093b06..ad4f604590c0 100644
> --- a/drivers/net/dsa/sja1105/sja1105_main.c
> +++ b/drivers/net/dsa/sja1105/sja1105_main.c
> @@ -766,12 +766,46 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
>  	return sja1105_clocking_setup_port(priv, port);
>  }
>  
> +/* The SJA1105 MAC programming model is through the static config (the xMII
> + * Mode table cannot be dynamically reconfigured), and we have to program
> + * that early (earlier than PHYLINK calls us, anyway).
> + * So just error out in case the connected PHY attempts to change the initial
> + * system interface MII protocol from what is defined in the DT, at least for
> + * now.
> + */
> +static bool sja1105_phy_mode_mismatch(struct sja1105_private *priv, int port,
> +				      phy_interface_t interface)
> +{
> +	struct sja1105_xmii_params_entry *mii;
> +	sja1105_phy_interface_t phy_mode;
> +
> +	mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
> +	phy_mode = mii->xmii_mode[port];
> +
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_MII:
> +		return (phy_mode != XMII_MODE_MII);
> +	case PHY_INTERFACE_MODE_RMII:
> +		return (phy_mode != XMII_MODE_RMII);
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +		return (phy_mode != XMII_MODE_RGMII);
> +	default:
> +		return true;
> +	}
> +}
> +
>  static void sja1105_mac_config(struct dsa_switch *ds, int port,
>  			       unsigned int link_an_mode,
>  			       const struct phylink_link_state *state)
>  {
>  	struct sja1105_private *priv = ds->priv;
>  
> +	if (sja1105_phy_mode_mismatch(priv, port, state->interface))
> +		return;
> +
>  	sja1105_adjust_port_config(priv, port, state->speed);
>  }
>  
> @@ -804,6 +838,19 @@ static void sja1105_phylink_validate(struct dsa_switch *ds, int port,
>  
>  	mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
>  
> +	/* include/linux/phylink.h says:
> +	 *     When @state->interface is %PHY_INTERFACE_MODE_NA, phylink
> +	 *     expects the MAC driver to return all supported link modes.
> +	 */
> +	if (state->interface != PHY_INTERFACE_MODE_NA &&
> +	    sja1105_phy_mode_mismatch(priv, port, state->interface)) {
> +		dev_warn(ds->dev, "PHY mode mismatch on port %d: "
> +			 "PHYLINK tried to change to %s\n",
> +			 port, phy_modes(state->interface));

Everything's fine except, please don't print to the kernel log for this.
You're just duplicating the prints in phylink.

> +		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +		return;
> +	}
> +
>  	/* The MAC does not support pause frames, and also doesn't
>  	 * support half-duplex traffic modes.
>  	 */
> -- 
> 2.17.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* [PATCH 36/87] net: eql.c: replace kmalloc and memset with kzalloc
From: Fuqian Huang @ 2019-06-27 17:39 UTC (permalink / raw)
  Cc: Fuqian Huang, David S. Miller, netdev, linux-kernel

kmalloc + memset(0) -> kzalloc

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/eql.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/eql.c b/drivers/net/eql.c
index 74263f8efe1a..2f101a6036e6 100644
--- a/drivers/net/eql.c
+++ b/drivers/net/eql.c
@@ -419,14 +419,13 @@ static int eql_enslave(struct net_device *master_dev, slaving_request_t __user *
 	if ((master_dev->flags & IFF_UP) == IFF_UP) {
 		/* slave is not a master & not already a slave: */
 		if (!eql_is_master(slave_dev) && !eql_is_slave(slave_dev)) {
-			slave_t *s = kmalloc(sizeof(*s), GFP_KERNEL);
+			slave_t *s = kzalloc(sizeof(*s), GFP_KERNEL);
 			equalizer_t *eql = netdev_priv(master_dev);
 			int ret;
 
 			if (!s)
 				return -ENOMEM;
 
-			memset(s, 0, sizeof(*s));
 			s->dev = slave_dev;
 			s->priority = srq.priority;
 			s->priority_bps = srq.priority;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 37/87] ethernet: atlx: remove memset after pci_alloc_consistent in atl1.c
From: Fuqian Huang @ 2019-06-27 17:39 UTC (permalink / raw)
  Cc: Fuqian Huang, Jay Cliburn, Chris Snook, David S. Miller,
	Michael Ellerman, Allison Randal, YueHaibing, Will Deacon,
	Thomas Gleixner, zhong jiang, Yang Wei, Colin Ian King, netdev,
	linux-kernel

pci_alloc_consistent calls dma_alloc_coherent directly.
In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 7c767ce9aafa..b5c6dc914720 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1060,8 +1060,6 @@ static s32 atl1_setup_ring_resources(struct atl1_adapter *adapter)
 		goto err_nomem;
 	}
 
-	memset(ring_header->desc, 0, ring_header->size);
-
 	/* init TPD ring */
 	tpd_ring->dma = ring_header->dma;
 	offset = (tpd_ring->dma & 0x7) ? (8 - (ring_header->dma & 0x7)) : 0;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 39/87] ethernet: bnxt: remove memset after dma_alloc_coherent
From: Fuqian Huang @ 2019-06-27 17:39 UTC (permalink / raw)
  Cc: Fuqian Huang, Michael Chan, David S. Miller, netdev, linux-kernel

In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index f758b2e0591f..1a51edb7de37 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2622,8 +2622,6 @@ static int bnxt_alloc_tx_rings(struct bnxt *bp)
 			mapping = txr->tx_push_mapping +
 				sizeof(struct tx_push_bd);
 			txr->data_mapping = cpu_to_le64(mapping);
-
-			memset(txr->tx_push, 0, sizeof(struct tx_push_bd));
 		}
 		qidx = bp->tc_to_qidx[j];
 		ring->queue_id = bp->q_info[qidx].queue_id;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 40/87] ethernet: cavium: replace vmalloc and memset with vzalloc
From: Fuqian Huang @ 2019-06-27 17:39 UTC (permalink / raw)
  Cc: Fuqian Huang, Derek Chickles, Satanand Burla, Felix Manlunas,
	David S. Miller, netdev, linux-kernel

vmalloc + memset(0) -> vzalloc

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c | 4 +---
 drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
index 43d11c38b38a..cf3835da32c8 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
@@ -719,12 +719,10 @@ static int cn23xx_setup_pf_mbox(struct octeon_device *oct)
 	for (i = 0; i < oct->sriov_info.max_vfs; i++) {
 		q_no = i * oct->sriov_info.rings_per_vf;
 
-		mbox = vmalloc(sizeof(*mbox));
+		mbox = vzalloc(sizeof(*mbox));
 		if (!mbox)
 			goto free_mbox;
 
-		memset(mbox, 0, sizeof(struct octeon_mbox));
-
 		spin_lock_init(&mbox->lock);
 
 		mbox->oct_dev = oct;
diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c b/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c
index fda49404968c..b3bd2767d3dd 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_vf_device.c
@@ -279,12 +279,10 @@ static int cn23xx_setup_vf_mbox(struct octeon_device *oct)
 {
 	struct octeon_mbox *mbox = NULL;
 
-	mbox = vmalloc(sizeof(*mbox));
+	mbox = vzalloc(sizeof(*mbox));
 	if (!mbox)
 		return 1;
 
-	memset(mbox, 0, sizeof(struct octeon_mbox));
-
 	spin_lock_init(&mbox->lock);
 
 	mbox->oct_dev = oct;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 38/87] ethernet: atlx: remove memset after pci_alloc_consistent in atl2.c
From: Fuqian Huang @ 2019-06-27 17:40 UTC (permalink / raw)
  Cc: Fuqian Huang, Jay Cliburn, Chris Snook, David S. Miller,
	Colin Ian King, Yang Wei, Allison Randal, Thomas Gleixner,
	Will Deacon, Mao Wenan, netdev, linux-kernel

pci_alloc_consitent calls dma_alloc_coherent directly.
In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index 3a3fb5ce0fee..3aba38322717 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -291,7 +291,6 @@ static s32 atl2_setup_ring_resources(struct atl2_adapter *adapter)
 		&adapter->ring_dma);
 	if (!adapter->ring_vir_addr)
 		return -ENOMEM;
-	memset(adapter->ring_vir_addr, 0, adapter->ring_size);
 
 	/* Init TXD Ring */
 	adapter->txd_dma = adapter->ring_dma ;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 87/87] ethernet: mlx4: remove memset after dma_alloc_coherent
From: Fuqian Huang @ 2019-06-27 17:42 UTC (permalink / raw)
  Cc: Fuqian Huang, Tariq Toukan, David S. Miller, netdev, linux-rdma,
	linux-kernel

In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/eq.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index a5be27772b8e..c790a5fcea73 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -1013,8 +1013,6 @@ static int mlx4_create_eq(struct mlx4_dev *dev, int nent,
 
 		dma_list[i] = t;
 		eq->page_list[i].map = t;
-
-		memset(eq->page_list[i].buf, 0, PAGE_SIZE);
 	}
 
 	eq->eqn = mlx4_bitmap_alloc(&priv->eq_table.bitmap);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 86/87] ethernet: mellanox:mlx4: replace kmalloc and memset with kzalloc
From: Fuqian Huang @ 2019-06-27 17:42 UTC (permalink / raw)
  Cc: Fuqian Huang, Tariq Toukan, David S. Miller, netdev, linux-rdma,
	linux-kernel

kmalloc + memset(0) -> kzalloc

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 6c01314e87b0..f1dff5c47676 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1062,7 +1062,7 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
 	struct mlx4_qp_context *context;
 	int err = 0;
 
-	context = kmalloc(sizeof(*context), GFP_KERNEL);
+	context = kzalloc(sizeof(*context), GFP_KERNEL);
 	if (!context)
 		return -ENOMEM;
 
@@ -1073,7 +1073,6 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
 	}
 	qp->event = mlx4_en_sqp_event;
 
-	memset(context, 0, sizeof(*context));
 	mlx4_en_fill_qp_context(priv, ring->actual_size, ring->stride, 0, 0,
 				qpn, ring->cqn, -1, context);
 	context->db_rec_addr = cpu_to_be64(ring->wqres.db.dma);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 85/87] ethernet: mellanox: mlx5: remove memset after kvzalloc
From: Fuqian Huang @ 2019-06-27 17:43 UTC (permalink / raw)
  Cc: Fuqian Huang, Saeed Mahameed, Leon Romanovsky, David S. Miller,
	netdev, linux-rdma, linux-kernel

kvzalloc already zeroes the memory.
So memset is unneeded.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.c          | 1 -
 drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 6a921e24cd5e..587c51fa3985 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -2391,7 +2391,6 @@ int mlx5_eswitch_get_vport_stats(struct mlx5_eswitch *esw,
 	MLX5_SET(query_vport_counter_in, in, vport_number, vport->vport);
 	MLX5_SET(query_vport_counter_in, in, other_vport, 1);
 
-	memset(out, 0, outlen);
 	err = mlx5_cmd_exec(esw->dev, in, sizeof(in), out, outlen);
 	if (err)
 		goto free_out;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 47b446d30f71..ef5fe3bd95f9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -993,7 +993,6 @@ static int esw_create_offloads_fdb_tables(struct mlx5_eswitch *esw, int nvports)
 	}
 
 	/* create send-to-vport group */
-	memset(flow_group_in, 0, inlen);
 	MLX5_SET(create_flow_group_in, flow_group_in, match_criteria_enable,
 		 MLX5_MATCH_MISC_PARAMETERS);
 
@@ -1151,7 +1150,6 @@ static int esw_create_vport_rx_group(struct mlx5_eswitch *esw, int nvports)
 		return -ENOMEM;
 
 	/* create vport rx group */
-	memset(flow_group_in, 0, inlen);
 	MLX5_SET(create_flow_group_in, flow_group_in, match_criteria_enable,
 		 MLX5_MATCH_MISC_PARAMETERS);
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH 84/87] ethernet: mellanox: mlxsw: remove memset after pci_alloc_persistent
From: Fuqian Huang @ 2019-06-27 17:43 UTC (permalink / raw)
  Cc: Fuqian Huang, Jiri Pirko, Ido Schimmel, David S. Miller, netdev,
	linux-kernel

pci_alloc_persistent calls dma_alloc_coherent directly.
In commit af7ddd8a627c
("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index b40455f8293d..be310ac0883a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -835,7 +835,6 @@ static int mlxsw_pci_queue_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 					     &mem_item->mapaddr);
 	if (!mem_item->buf)
 		return -ENOMEM;
-	memset(mem_item->buf, 0, mem_item->size);
 
 	q->elem_info = kcalloc(q->count, sizeof(*q->elem_info), GFP_KERNEL);
 	if (!q->elem_info) {
-- 
2.11.0


^ permalink raw reply related


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