Netdev List
 help / color / mirror / Atom feed
* [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Xin Long @ 2019-02-25 11:25 UTC (permalink / raw)
  To: linux-kernel, network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Neil Horman

sctp_hdr(skb) only works when skb->transport_header is set properly.

But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()

skb->transport_header is not guaranteed to be right value for sctp.
It will cause to fail to check the checksum for sctp packets.

So fix it by using offset, which is always right in all places.

Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/checksum.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
index 32ee65a..1c6e6c0 100644
--- a/include/net/sctp/checksum.h
+++ b/include/net/sctp/checksum.h
@@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
 static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
 					unsigned int offset)
 {
-	struct sctphdr *sh = sctp_hdr(skb);
+	struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
 	const struct skb_checksum_ops ops = {
 		.update  = sctp_csum_update,
 		.combine = sctp_csum_combine,
-- 
2.1.0


^ permalink raw reply related

* [PATCH net] ipvs: get sctphdr by sctphoff in sctp_csum_check
From: Xin Long @ 2019-02-25 11:27 UTC (permalink / raw)
  To: network dev, netfilter-devel; +Cc: Marcelo Ricardo Leitner, Neil Horman, pablo

sctp_csum_check() is called by sctp_s/dnat_handler() where it calls
skb_make_writable() to ensure sctphdr to be linearized.

So there's no need to get sctphdr by calling skb_header_pointer()
in sctp_csum_check().

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/ipvs/ip_vs_proto_sctp.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index b0cd7d0..0ecf241 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -183,7 +183,7 @@ static int
 sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
 {
 	unsigned int sctphoff;
-	struct sctphdr *sh, _sctph;
+	struct sctphdr *sh;
 	__le32 cmp, val;
 
 #ifdef CONFIG_IP_VS_IPV6
@@ -193,10 +193,7 @@ sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
 #endif
 		sctphoff = ip_hdrlen(skb);
 
-	sh = skb_header_pointer(skb, sctphoff, sizeof(_sctph), &_sctph);
-	if (sh == NULL)
-		return 0;
-
+	sh = (struct sctphdr *)(skb->data + sctphoff);
 	cmp = sh->checksum;
 	val = sctp_compute_cksum(skb, sctphoff);
 
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH net v2 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
From: Stefano Brivio @ 2019-02-25 11:35 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <6d6f6a2512743d6e1968dbfe765eb85b478bd033.1551093086.git.pabeni@redhat.com>

Just three minor details, feel free to ignore:

On Mon, 25 Feb 2019 12:13:46 +0100
Paolo Abeni <pabeni@redhat.com> wrote:

> @@ -1006,6 +1017,60 @@ test_pmtu_vti6_link_change_mtu() {
>  	return ${fail}
>  }
>  
> +chk_command() {

All the other functions checking something are called 'check_*'.

> +	cmd=${1}
> +
> +	if ! which ${cmd} > /dev/null 2>&1; then
> +		err "  missing required command: '${cmd}'"
> +		return 1
> +	fi
> +	return 0
> +}
> +
> +test_cleanup_vxlanX_exception() {
> +	outer="${1}"
> +	encap="vxlan"
> +	ll_mtu=4000
> +
> +	chk_command taskset || return 2
> +	chk_command timeout || return 2
> +	cpu_list=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2)
> +
> +	setup namespaces routing ${encap}${outer} || return 2
> +	trace "${ns_a}" ${encap}_a   "${ns_b}"  ${encap}_b \
> +	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
> +	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B
> +
> +	# Create route exception by exceeding link layer MTU
> +	mtu "${ns_a}"  veth_A-R1 $((${ll_mtu} + 1000))
> +	mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
> +	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
> +	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
> +
> +	mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
> +	mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
> +
> +	# Fill exception cache for multiple CPUs (2)
> +	# we can always use inner IPv4 for that
> +	for cpu in ${cpu_list}; do
> +		taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
> +	done
> +
> +	if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then
> +		err "  can't delete veth device in a timely manner, PMTU dst likely leaked"
> +		return 1
> +	fi
> +	return 0

No need for explicit return 0.

> +}
> +
> +test_pmtu_ipv6_exception_cleanup() {
> +	test_cleanup_vxlanX_exception 6 vxlan
> +}
> +
> +test_pmtu_ipv4_exception_cleanup() {
> +	test_cleanup_vxlanX_exception 4 vxlan

This function now takes just one argument.

-- 
Stefano

^ permalink raw reply

* [PATCH net-next resend 0/2]
From: Marek Behún @ 2019-02-25 11:39 UTC (permalink / raw)
  To: netdev; +Cc: Andrew Lunn, Florian Fainelli, David Miller

Hi,
this is a rebased resend of patches I sent one month ago.
I added Reviewed-by tags by Andrew and Florian.

Marek



^ permalink raw reply

* [PATCH net-next resend 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X
From: Marek Behún @ 2019-02-25 11:39 UTC (permalink / raw)
  To: netdev; +Cc: Andrew Lunn, Florian Fainelli, David Miller, Marek Behún
In-Reply-To: <20190225113955.25700-1-marek.behun@nic.cz>

Commit 787799a9d555 sets the SERDES interfaces of 6390 and 6390X to
1000BaseX, but this is only needed on 6390X, since there are SERDES
interfaces which can be used on lower ports on 6390.

This commit fixes this by returning to previous behaviour on 6390.
(Previous behaviour means that CMODE is not set at all if requested mode
is NA).

This is needed on Turris MOX, where the 88e6190 is connected to CPU in
2500BaseX mode.

Fixes: 787799a9d555 ("net: dsa: mv88e6xxx: Default ports 9/10 6390X CMODE to 1000BaseX")
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/port.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index ebd26b6a93e6..ee7029f4ee22 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -444,6 +444,8 @@ int mv88e6390_port_set_cmode(struct mv88e6xxx_chip *chip, int port,
 			     phy_interface_t mode)
 {
 	switch (mode) {
+	case PHY_INTERFACE_MODE_NA:
+		return 0;
 	case PHY_INTERFACE_MODE_XGMII:
 	case PHY_INTERFACE_MODE_XAUI:
 	case PHY_INTERFACE_MODE_RXAUI:
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next resend 2/2] net: dsa: mv88e6xxx: Fix phylink_validate for Topaz family
From: Marek Behún @ 2019-02-25 11:39 UTC (permalink / raw)
  To: netdev; +Cc: Andrew Lunn, Florian Fainelli, David Miller, Marek Behún
In-Reply-To: <20190225113955.25700-1-marek.behun@nic.cz>

The Topaz family should have different phylink_validate method from the
Peridot, since on Topaz the port supporting 2500BaseX mode is port 5,
not 9 and 10.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d30336f259ce..f223e6476369 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -658,6 +658,20 @@ static void mv88e6185_phylink_validate(struct mv88e6xxx_chip *chip, int port,
 	mv88e6065_phylink_validate(chip, port, mask, state);
 }
 
+static void mv88e6341_phylink_validate(struct mv88e6xxx_chip *chip, int port,
+				       unsigned long *mask,
+				       struct phylink_link_state *state)
+{
+	if (port >= 5)
+		phylink_set(mask, 2500baseX_Full);
+
+	/* No ethtool bits for 200Mbps */
+	phylink_set(mask, 1000baseT_Full);
+	phylink_set(mask, 1000baseX_Full);
+
+	mv88e6065_phylink_validate(chip, port, mask, state);
+}
+
 static void mv88e6352_phylink_validate(struct mv88e6xxx_chip *chip, int port,
 				       unsigned long *mask,
 				       struct phylink_link_state *state)
@@ -3080,7 +3094,7 @@ static const struct mv88e6xxx_ops mv88e6141_ops = {
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.serdes_power = mv88e6341_serdes_power,
 	.gpio_ops = &mv88e6352_gpio_ops,
-	.phylink_validate = mv88e6390_phylink_validate,
+	.phylink_validate = mv88e6341_phylink_validate,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3712,7 +3726,7 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.avb_ops = &mv88e6390_avb_ops,
 	.ptp_ops = &mv88e6352_ptp_ops,
-	.phylink_validate = mv88e6390_phylink_validate,
+	.phylink_validate = mv88e6341_phylink_validate,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Michal Kubecek @ 2019-02-25 11:48 UTC (permalink / raw)
  To: Tariq Toukan; +Cc: John W. Linville, netdev, Eran Ben Elisha, Aya Levin
In-Reply-To: <20190224181104.GA1914@unicorn.suse.cz>

On Sun, Feb 24, 2019 at 07:11:04PM +0100, Michal Kubecek wrote:
> On Sun, Feb 24, 2019 at 05:08:21PM +0200, Tariq Toukan wrote:
> > From: Aya Levin <ayal@mellanox.com>
> > 
> > Introduce 50Gbps per lane link modes and 200Gbps speed, update print
> > functions and initialization functions accordingly.
> > In addition, update related man page accordingly.
> > 
> > Signed-off-by: Aya Levin <ayal@mellanox.com>
> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > ---
> >  ethtool-copy.h | 19 ++++++++++++++++++-
> >  ethtool.8.in   | 15 +++++++++++++++
> >  ethtool.c      | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 85 insertions(+), 1 deletion(-)
> > 
> > diff --git a/ethtool-copy.h b/ethtool-copy.h
> > index 6bfbb85f9402..de459900b2d3 100644
> > --- a/ethtool-copy.h
> > +++ b/ethtool-copy.h
> > @@ -1455,15 +1455,31 @@ enum ethtool_link_mode_bit_indices {
> >  	ETHTOOL_LINK_MODE_FEC_NONE_BIT	= 49,
> >  	ETHTOOL_LINK_MODE_FEC_RS_BIT	= 50,
> >  	ETHTOOL_LINK_MODE_FEC_BASER_BIT	= 51,
> > +	ETHTOOL_LINK_MODE_50000baseKR_Full_BIT			= 52,
> > +	ETHTOOL_LINK_MODE_50000baseSR_Full_BIT			= 53,
> > +	ETHTOOL_LINK_MODE_50000baseCR_Full_BIT			= 54,
> > +	ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT		= 55,
> > +	ETHTOOL_LINK_MODE_50000baseDR_Full_BIT			= 56,
> > +	ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT		= 57,
> > +	ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT		= 58,
> > +	ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT		= 59,
> > +	ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT	= 60,
> > +	ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT		= 61,
> > +	ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT		= 62,
> > +	ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT		= 63,
> > +	ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT	= 64,
> > +	ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT		= 65,
> > +	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT		= 66,
> >  
> >  	/* Last allowed bit for __ETHTOOL_LINK_MODE_LEGACY_MASK is bit
> >  	 * 31. Please do NOT define any SUPPORTED_* or ADVERTISED_*
> >  	 * macro for bits > 31. The only way to use indices > 31 is to
> >  	 * use the new ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS API.
> >  	 */
> > +	ETHTOOL_LINK_MODE_LAST,
> >  
> >  	__ETHTOOL_LINK_MODE_LAST
> > -	  = ETHTOOL_LINK_MODE_FEC_BASER_BIT,
> > +	  = ETHTOOL_LINK_MODE_LAST - 1,
> >  };
> 
> The name ETHTOOL_LINK_MODE_LAST is a bit misleading, maybe it should
> rather be called ETHTOOL_LINK_MODE_COUNT. Also, there should be
> parentheses around the expression to avoid surprises when
> __ETHTOOL_LINK_MODE_LAST is used in an expression.
> 
> But this change is not in kernel include/uapi/linux/ethtool.h in
> mainline, net or net-next. As ethtool-copy.h is supposed to be a copy of
> sanitized kernel uapi header, you should make the change there first and
> then sync the header to ethtool.

For the record, net-next tree now has simplified this even more with
commit e728fdf06289 ("net: phy: improve definition of
__ETHTOOL_LINK_MODE_MASK_NBITS").

Michal Kubecek

^ permalink raw reply

* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Anand Moon @ 2019-02-25 12:19 UTC (permalink / raw)
  To: Martin Blumenstingl, Bartosz Golaszewski; +Cc: linux-amlogic, netdev
In-Reply-To: <CAFBinCDRKJS-RXgVBDo2gjp_bU8Wmbhr+HGXNad4vCFyrvN_HA@mail.gmail.com>

hi Martin,

+Bartosz Golaszewski <bgolaszewski@baylibre.com>

On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> I have seen Anand's your question in [0]:
> > only issue is I have is the each time their is random MAC address so I
> > get new IP from dhcp server.
> > How can I avoid this. I have tried to enable eFuse driver but with no success.
>
> u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> pass it (via the .dtb) to the kernel.
> This requires an ethernet0 alias in the mainline .dts though, see [1]
> for and example.
>
> I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> if it doesn't then there's a nvmem-cells binding for all Ethernet
> controllers: [2] (please note that the function implementing this
> binding was recently renamed: [3])
> as far as I can tell the stmmac driver doesn't support the nvmem-cells
> based binding yet
>
> Anand, if you want to work on this: feel free to do so!
> I have the SDHC MMC driver and a discussion about the power-domain
> drivers on my TODO-list, so I'm pretty busy at the moment.
>
>
> Regards
> Martin
>

Thanks for your inputs :) 8)

After enable CONFIG_MESON_MX_EFUSE and added the alias

On Odroid C1+
# hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
0000000 1143 0000 4b48 3143 3131 3232 3346 4537
0000010 3942 4432 0000 0000 0000 0000 0000 0000
0000020 0000 0000 0000 0000 0000 0000 0000 0000
*
00001b0 0000 0000 1e00 1006 addc 0000 0000 0000    *00:1e:06:10:dc:ad*
mac address from nvmem
00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
00001d0 3732 3130 6237 6537 3165 6437 3034 3764
00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
0000200

as per my understating on 3.10.x the mac address is also set in the u-boot env.

odroidc#printenv ethaddr
ethaddr=00:1e:06:10:dc:ad

Which is equal to above nvmem setting above.

* But sill I am getting new mac address on every reboot. :p *

Odroid C2:
I could clearly observe the same.

[root@archl-c2l alarm]# hexdump /sys/devices/platform/efuse/efuse0/nvmem
0000000 0000 0000 0000 0000 0000 0000 0000 0000
0000010 0000 0000 4b48 3243 3331 3532 4434 4346
0000020 4145 4146 0000 0000 0000 0000 0000 0000
0000030 0000 0000 1e00 3306 7a37 0000 0000 0000     *00:1e:06:33:37:7a*
0000040 0000 0000 0000 4b48 3243 3132 3631 3430
0000050 3834 3932 3631 3837 3937 3466 6233 6366
0000060 357c c7ec cc98 0f6d 65b7 92cf 000b 0a00
0000070 0000 0000 0000 0000 0000 0000 0000 0000
0000080 0014 0000 0000 0000 0000 0000 0000 0000
0000090 0000 0000 0000 0000 0000 0000 0000 0000
*
00000c0
[root@archl-c2l alarm]# ifconfig
eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 10.0.0.140  netmask 255.255.255.0  broadcast 10.0.0.255
        inet6 2002:ac10:e303:e472:21e:6ff:fe33:377a  prefixlen 64
scopeid 0x0<global>
        inet6 fe80::21e:6ff:fe33:377a  prefixlen 64  scopeid 0x20<link>
        ether 00:1e:06:33:37:7a  txqueuelen 1000  (Ethernet)
        RX packets 70485  bytes 4390622 (4.1 MiB)
        RX errors 0  dropped 28  overruns 0  frame 0
        TX packets 6159  bytes 766939 (748.9 KiB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0
        device interrupt 18

How can we read the mac address from given offset in nvmem.
>
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2019-February/010464.html
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7c36209c46c4d162202b65eed2e66962ad8c3c1
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9217e566bdee4583d0a9ea4879c8f5e004886eac
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=afa64a72b862a7a9d04f8d07fba632eaf06b23f8

Yes thanks for the inputs.

I see c85efcc60a892210aa10688d5de1f997d5cad799 ("ARM: davinci:
da830-evm: use cell nvmem lookup for mac address")
I do not know what this changes will help get the mac address from nvmem region.

If some body could share some inputs it will be better for me to debug.

Best Regards
-Anand

^ permalink raw reply

* [PATCH net-next] net: sched: act_tunnel_key: fix metadata handling
From: Vlad Buslov @ 2019-02-25 12:21 UTC (permalink / raw)
  To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, dcaratti, wenxu, roid,
	Vlad Buslov

Tunnel key action params->tcft_enc_metadata is only set when action is
TCA_TUNNEL_KEY_ACT_SET. However, metadata pointer is incorrectly
dereferenced during tunnel key init and release without verifying that
action is if correct type, which causes NULL pointer dereference. Metadata
tunnel dst_cache is also leaked on action overwrite.

Fix metadata handling:
- Verify that metadata pointer is not NULL before dereferencing it in
  tunnel_key_init error handling code.
- Move dst_cache destroy code into tunnel_key_release_params() function
  that is called in both action overwrite and release cases (fixes resource
  leak) and verifies that actions has correct type before dereferencing
  metadata pointer (fixes NULL pointer dereference).

Oops with KASAN enabled during tdc tests execution:

[  261.080482] ==================================================================
[  261.088049] BUG: KASAN: null-ptr-deref in dst_cache_destroy+0x21/0xa0
[  261.094613] Read of size 8 at addr 00000000000000b0 by task tc/2976
[  261.102524] CPU: 14 PID: 2976 Comm: tc Not tainted 5.0.0-rc7+ #157
[  261.108844] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  261.116726] Call Trace:
[  261.119234]  dump_stack+0x9a/0xeb
[  261.122625]  ? dst_cache_destroy+0x21/0xa0
[  261.126818]  ? dst_cache_destroy+0x21/0xa0
[  261.131004]  kasan_report+0x176/0x192
[  261.134752]  ? idr_get_next+0xd0/0x120
[  261.138578]  ? dst_cache_destroy+0x21/0xa0
[  261.142768]  dst_cache_destroy+0x21/0xa0
[  261.146799]  tunnel_key_release+0x3a/0x50 [act_tunnel_key]
[  261.152392]  tcf_action_cleanup+0x2c/0xc0
[  261.156490]  tcf_generic_walker+0x4c2/0x5c0
[  261.160794]  ? tcf_action_dump_1+0x390/0x390
[  261.165163]  ? tunnel_key_walker+0x5/0x1a0 [act_tunnel_key]
[  261.170865]  ? tunnel_key_walker+0xe9/0x1a0 [act_tunnel_key]
[  261.176641]  tca_action_gd+0x600/0xa40
[  261.180482]  ? tca_get_fill.constprop.17+0x200/0x200
[  261.185548]  ? __lock_acquire+0x588/0x1d20
[  261.189741]  ? __lock_acquire+0x588/0x1d20
[  261.193922]  ? mark_held_locks+0x90/0x90
[  261.197944]  ? mark_held_locks+0x90/0x90
[  261.202018]  ? __nla_parse+0xfe/0x190
[  261.205774]  tc_ctl_action+0x218/0x230
[  261.209614]  ? tcf_action_add+0x230/0x230
[  261.213726]  rtnetlink_rcv_msg+0x3a5/0x600
[  261.217910]  ? lock_downgrade+0x2d0/0x2d0
[  261.222006]  ? validate_linkmsg+0x400/0x400
[  261.226278]  ? find_held_lock+0x6d/0xd0
[  261.230200]  ? match_held_lock+0x1b/0x210
[  261.234296]  ? validate_linkmsg+0x400/0x400
[  261.238567]  netlink_rcv_skb+0xc7/0x1f0
[  261.242489]  ? netlink_ack+0x470/0x470
[  261.246319]  ? netlink_deliver_tap+0x1f3/0x5a0
[  261.250874]  netlink_unicast+0x2ae/0x350
[  261.254884]  ? netlink_attachskb+0x340/0x340
[  261.261647]  ? _copy_from_iter_full+0xdd/0x380
[  261.268576]  ? __virt_addr_valid+0xb6/0xf0
[  261.275227]  ? __check_object_size+0x159/0x240
[  261.282184]  netlink_sendmsg+0x4d3/0x630
[  261.288572]  ? netlink_unicast+0x350/0x350
[  261.295132]  ? netlink_unicast+0x350/0x350
[  261.301608]  sock_sendmsg+0x6d/0x80
[  261.307467]  ___sys_sendmsg+0x48e/0x540
[  261.313633]  ? copy_msghdr_from_user+0x210/0x210
[  261.320545]  ? save_stack+0x89/0xb0
[  261.326289]  ? __lock_acquire+0x588/0x1d20
[  261.332605]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  261.340063]  ? mark_held_locks+0x90/0x90
[  261.346162]  ? do_filp_open+0x138/0x1d0
[  261.352108]  ? may_open_dev+0x50/0x50
[  261.357897]  ? match_held_lock+0x1b/0x210
[  261.364016]  ? __fget_light+0xa6/0xe0
[  261.369840]  ? __sys_sendmsg+0xd2/0x150
[  261.375814]  __sys_sendmsg+0xd2/0x150
[  261.381610]  ? __ia32_sys_shutdown+0x30/0x30
[  261.388026]  ? lock_downgrade+0x2d0/0x2d0
[  261.394182]  ? mark_held_locks+0x1c/0x90
[  261.400230]  ? do_syscall_64+0x1e/0x280
[  261.406172]  do_syscall_64+0x78/0x280
[  261.411932]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  261.419103] RIP: 0033:0x7f28e91a8b87
[  261.424791] Code: 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 80 00 00 00 00 8b 05 6a 2b 2c 00 48 63 d2 48 63 ff 85 c0 75 18 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 59 f3 c3 0f 1f 80 00 00 00 00 53 48 89 f3 48
[  261.448226] RSP: 002b:00007ffdc5c4e2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  261.458183] RAX: ffffffffffffffda RBX: 000000005c73c202 RCX: 00007f28e91a8b87
[  261.467728] RDX: 0000000000000000 RSI: 00007ffdc5c4e340 RDI: 0000000000000003
[  261.477342] RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000c
[  261.486970] R10: 000000000000000c R11: 0000000000000246 R12: 0000000000000001
[  261.496599] R13: 000000000067b4e0 R14: 00007ffdc5c5248c R15: 00007ffdc5c52480
[  261.506281] ==================================================================
[  261.516076] Disabling lock debugging due to kernel taint
[  261.523979] BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
[  261.534413] #PF error: [normal kernel read fault]
[  261.541730] PGD 8000000317400067 P4D 8000000317400067 PUD 316878067 PMD 0
[  261.551294] Oops: 0000 [#1] SMP KASAN PTI
[  261.557985] CPU: 14 PID: 2976 Comm: tc Tainted: G    B             5.0.0-rc7+ #157
[  261.568306] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  261.578874] RIP: 0010:dst_cache_destroy+0x21/0xa0
[  261.586413] Code: f4 ff ff ff eb f6 0f 1f 00 0f 1f 44 00 00 41 56 41 55 49 c7 c6 60 fe 35 af 41 54 55 49 89 fc 53 bd ff ff ff ff e8 ef 98 73 ff <49> 83 3c 24 00 75 35 eb 6c 4c 63 ed e8 de 98 73 ff 4a 8d 3c ed 40
[  261.611247] RSP: 0018:ffff888316447160 EFLAGS: 00010282
[  261.619564] RAX: 0000000000000000 RBX: ffff88835b3e2f00 RCX: ffffffffad1c5071
[  261.629862] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: 0000000000000297
[  261.640149] RBP: 00000000ffffffff R08: fffffbfff5dd4e89 R09: fffffbfff5dd4e89
[  261.650467] R10: 0000000000000001 R11: fffffbfff5dd4e88 R12: 00000000000000b0
[  261.660785] R13: ffff8883267a10c0 R14: ffffffffaf35fe60 R15: 0000000000000001
[  261.671110] FS:  00007f28ea3e6400(0000) GS:ffff888364200000(0000) knlGS:0000000000000000
[  261.682447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  261.691491] CR2: 00000000000000b0 CR3: 00000003178ae004 CR4: 00000000001606e0
[  261.701283] Call Trace:
[  261.706374]  tunnel_key_release+0x3a/0x50 [act_tunnel_key]
[  261.714522]  tcf_action_cleanup+0x2c/0xc0
[  261.721208]  tcf_generic_walker+0x4c2/0x5c0
[  261.728074]  ? tcf_action_dump_1+0x390/0x390
[  261.734996]  ? tunnel_key_walker+0x5/0x1a0 [act_tunnel_key]
[  261.743247]  ? tunnel_key_walker+0xe9/0x1a0 [act_tunnel_key]
[  261.751557]  tca_action_gd+0x600/0xa40
[  261.757991]  ? tca_get_fill.constprop.17+0x200/0x200
[  261.765644]  ? __lock_acquire+0x588/0x1d20
[  261.772461]  ? __lock_acquire+0x588/0x1d20
[  261.779266]  ? mark_held_locks+0x90/0x90
[  261.785880]  ? mark_held_locks+0x90/0x90
[  261.792470]  ? __nla_parse+0xfe/0x190
[  261.798738]  tc_ctl_action+0x218/0x230
[  261.805145]  ? tcf_action_add+0x230/0x230
[  261.811760]  rtnetlink_rcv_msg+0x3a5/0x600
[  261.818564]  ? lock_downgrade+0x2d0/0x2d0
[  261.825433]  ? validate_linkmsg+0x400/0x400
[  261.832256]  ? find_held_lock+0x6d/0xd0
[  261.838624]  ? match_held_lock+0x1b/0x210
[  261.845142]  ? validate_linkmsg+0x400/0x400
[  261.851729]  netlink_rcv_skb+0xc7/0x1f0
[  261.857976]  ? netlink_ack+0x470/0x470
[  261.864132]  ? netlink_deliver_tap+0x1f3/0x5a0
[  261.870969]  netlink_unicast+0x2ae/0x350
[  261.877294]  ? netlink_attachskb+0x340/0x340
[  261.883962]  ? _copy_from_iter_full+0xdd/0x380
[  261.890750]  ? __virt_addr_valid+0xb6/0xf0
[  261.897188]  ? __check_object_size+0x159/0x240
[  261.903928]  netlink_sendmsg+0x4d3/0x630
[  261.910112]  ? netlink_unicast+0x350/0x350
[  261.916410]  ? netlink_unicast+0x350/0x350
[  261.922656]  sock_sendmsg+0x6d/0x80
[  261.928257]  ___sys_sendmsg+0x48e/0x540
[  261.934183]  ? copy_msghdr_from_user+0x210/0x210
[  261.940865]  ? save_stack+0x89/0xb0
[  261.946355]  ? __lock_acquire+0x588/0x1d20
[  261.952358]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  261.959468]  ? mark_held_locks+0x90/0x90
[  261.965248]  ? do_filp_open+0x138/0x1d0
[  261.970910]  ? may_open_dev+0x50/0x50
[  261.976386]  ? match_held_lock+0x1b/0x210
[  261.982210]  ? __fget_light+0xa6/0xe0
[  261.987648]  ? __sys_sendmsg+0xd2/0x150
[  261.993263]  __sys_sendmsg+0xd2/0x150
[  261.998613]  ? __ia32_sys_shutdown+0x30/0x30
[  262.004555]  ? lock_downgrade+0x2d0/0x2d0
[  262.010236]  ? mark_held_locks+0x1c/0x90
[  262.015758]  ? do_syscall_64+0x1e/0x280
[  262.021234]  do_syscall_64+0x78/0x280
[  262.026500]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  262.033207] RIP: 0033:0x7f28e91a8b87
[  262.038421] Code: 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 80 00 00 00 00 8b 05 6a 2b 2c 00 48 63 d2 48 63 ff 85 c0 75 18 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 59 f3 c3 0f 1f 80 00 00 00 00 53 48 89 f3 48
[  262.060708] RSP: 002b:00007ffdc5c4e2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  262.070112] RAX: ffffffffffffffda RBX: 000000005c73c202 RCX: 00007f28e91a8b87
[  262.079087] RDX: 0000000000000000 RSI: 00007ffdc5c4e340 RDI: 0000000000000003
[  262.088122] RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000c
[  262.097157] R10: 000000000000000c R11: 0000000000000246 R12: 0000000000000001
[  262.106207] R13: 000000000067b4e0 R14: 00007ffdc5c5248c R15: 00007ffdc5c52480
[  262.115271] Modules linked in: act_tunnel_key act_skbmod act_simple act_connmark nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 act_csum libcrc32c act_meta_skbtcindex act_meta_skbprio act_meta_mark act_ife ife act_police act_sample psample act_gact veth nfsv3 nfs_acl nfs lockd grace fscache bridge stp llc intel_rapl sb_edac mlx5_ib x86_pkg_temp_thermal sunrpc intel_powerclamp coretemp ib_uverbs kvm_intel ib_core kvm irqbypass mlx5_core crct10dif_pclmul crc32_pclmul crc32c_intel igb ghash_clmulni_intel intel_cstate mlxfw iTCO_wdt devlink intel_uncore iTCO_vendor_support ipmi_ssif ptp mei_me intel_rapl_perf ioatdma joydev pps_core ses mei i2c_i801 pcspkr enclosure lpc_ich dca wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad acpi_power_meter pcc_cpufreq ast i2c_algo_bit drm_kms_helper ttm drm mpt3sas raid_class scsi_transport_sas
[  262.204393] CR2: 00000000000000b0
[  262.210390] ---[ end trace 2e41d786f2c7901a ]---
[  262.226790] RIP: 0010:dst_cache_destroy+0x21/0xa0
[  262.234083] Code: f4 ff ff ff eb f6 0f 1f 00 0f 1f 44 00 00 41 56 41 55 49 c7 c6 60 fe 35 af 41 54 55 49 89 fc 53 bd ff ff ff ff e8 ef 98 73 ff <49> 83 3c 24 00 75 35 eb 6c 4c 63 ed e8 de 98 73 ff 4a 8d 3c ed 40
[  262.258311] RSP: 0018:ffff888316447160 EFLAGS: 00010282
[  262.266304] RAX: 0000000000000000 RBX: ffff88835b3e2f00 RCX: ffffffffad1c5071
[  262.276251] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: 0000000000000297
[  262.286208] RBP: 00000000ffffffff R08: fffffbfff5dd4e89 R09: fffffbfff5dd4e89
[  262.296183] R10: 0000000000000001 R11: fffffbfff5dd4e88 R12: 00000000000000b0
[  262.306157] R13: ffff8883267a10c0 R14: ffffffffaf35fe60 R15: 0000000000000001
[  262.316139] FS:  00007f28ea3e6400(0000) GS:ffff888364200000(0000) knlGS:0000000000000000
[  262.327146] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  262.335815] CR2: 00000000000000b0 CR3: 00000003178ae004 CR4: 00000000001606e0

Fixes: ee28bb56ac5b ("net/sched: fix memory leak in act_tunnel_key_init()")
Fixes: 41411e2fd6b8 ("net/sched: act_tunnel_key: Add dst_cache support")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 net/sched/act_tunnel_key.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 3404af72b4c1..2a5f215ae876 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -201,8 +201,14 @@ static void tunnel_key_release_params(struct tcf_tunnel_key_params *p)
 {
 	if (!p)
 		return;
-	if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET)
+	if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET) {
+#ifdef CONFIG_DST_CACHE
+		struct ip_tunnel_info *info = &p->tcft_enc_metadata->u.tun_info;
+
+		dst_cache_destroy(&info->dst_cache);
+#endif
 		dst_release(&p->tcft_enc_metadata->dst);
+	}
 	kfree_rcu(p, rcu);
 }
 
@@ -384,10 +390,12 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 
 release_dst_cache:
 #ifdef CONFIG_DST_CACHE
-	dst_cache_destroy(&metadata->u.tun_info.dst_cache);
+	if (metadata)
+		dst_cache_destroy(&metadata->u.tun_info.dst_cache);
 #endif
 release_tun_meta:
-	dst_release(&metadata->dst);
+	if (metadata)
+		dst_release(&metadata->dst);
 
 err_out:
 	if (exists)
@@ -401,15 +409,8 @@ static void tunnel_key_release(struct tc_action *a)
 {
 	struct tcf_tunnel_key *t = to_tunnel_key(a);
 	struct tcf_tunnel_key_params *params;
-#ifdef CONFIG_DST_CACHE
-	struct ip_tunnel_info *info;
-#endif
 
 	params = rcu_dereference_protected(t->params, 1);
-#ifdef CONFIG_DST_CACHE
-	info = &params->tcft_enc_metadata->u.tun_info;
-	dst_cache_destroy(&info->dst_cache);
-#endif
 	tunnel_key_release_params(params);
 }
 
-- 
2.13.6


^ permalink raw reply related

* [PATCH] cfg80211: Use kmemdup in cfg80211_gen_new_ie()
From: YueHaibing @ 2019-02-25 12:38 UTC (permalink / raw)
  To: Johannes Berg; +Cc: YueHaibing, linux-wireless, netdev, kernel-janitors

Use kmemdup rather than duplicating its implementation

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/wireless/scan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 287518c6caa4..04d888628f29 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -190,10 +190,9 @@ static size_t cfg80211_gen_new_ie(const u8 *ie, size_t ielen,
 	/* copy subelement as we need to change its content to
 	 * mark an ie after it is processed.
 	 */
-	sub_copy = kmalloc(subie_len, gfp);
+	sub_copy = kmemdup(subelement, subie_len, gfp);
 	if (!sub_copy)
 		return 0;
-	memcpy(sub_copy, subelement, subie_len);
 
 	pos = &new_ie[0];




^ permalink raw reply related

* Re: [PATCH net v2 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
From: Sabrina Dubroca @ 2019-02-25 12:33 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern, Stefano Brivio
In-Reply-To: <6d6f6a2512743d6e1968dbfe765eb85b478bd033.1551093086.git.pabeni@redhat.com>

2019-02-25, 12:13:46 +0100, Paolo Abeni wrote:
> +	if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then

That doesn't work. "ip link del" is stuck in a way that timeout can't
terminate it, so this is still going to hang. Did you actually test
this? :/

> +		err "  can't delete veth device in a timely manner, PMTU dst likely leaked"
> +		return 1
> +	fi
> +	return 0
> +}

-- 
Sabrina

^ permalink raw reply

* Re: [PATCH ethtool] ethtool: Add support for 200Gbps (50Gbps per lane) link mode
From: Aya Levin @ 2019-02-25 12:42 UTC (permalink / raw)
  To: Michal Kubecek, Andrew Lunn
  Cc: Tariq Toukan, John W. Linville, netdev@vger.kernel.org,
	Eran Ben Elisha
In-Reply-To: <20190224184046.GB1914@unicorn.suse.cz>



On 2/24/2019 8:40 PM, Michal Kubecek wrote:
> On Sun, Feb 24, 2019 at 05:47:51PM +0100, Andrew Lunn wrote:
>> On Sun, Feb 24, 2019 at 05:08:21PM +0200, Tariq Toukan wrote:
>>> From: Aya Levin <ayal@mellanox.com>
>>> index 5a26cff5fb33..64ce0711ad5f 100644
>>> --- a/ethtool.8.in
>>> +++ b/ethtool.8.in
>>> @@ -650,6 +650,11 @@ lB	l	lB.
>>>   0x400000000	50000baseCR2 Full
>>>   0x800000000	50000baseKR2 Full
>>>   0x10000000000	50000baseSR2 Full
>>> +0x10000000000000	50000baseKR Full
>>> +0x20000000000000	50000baseSR Full
>>> +0x40000000000000	50000baseCR Full
>>> +0x80000000000000	50000baseLR_ER_FR Full
>>> +0x100000000000000	50000baseDR Full
>>>   0x8000000	56000baseKR4 Full
>>>   0x10000000	56000baseCR4 Full
>>>   0x20000000	56000baseSR4 Full
>>> @@ -658,6 +663,16 @@ lB	l	lB.
>>>   0x2000000000	100000baseSR4 Full
>>>   0x4000000000	100000baseCR4 Full
>>>   0x8000000000	100000baseLR4_ER4 Full
>>> +0x200000000000000	100000baseKR2 Full
>>> +0x400000000000000	100000baseSR2 Full
>>> +0x800000000000000	100000baseCR2 Full
>>> +0x1000000000000000	100000baseLR2_ER2_FR2 Full
>>> +0x2000000000000000	100000baseDR2 Full
>>> +0x4000000000000000	200000baseKR4 Full
>>> +0x8000000000000000	200000baseSR4 Full
>>> +0x10000000000000000	200000baseLR4_ER4_FR4 Full
>>> +0x20000000000000000	200000baseDR4 Full
>>> +0x40000000000000000	200000baseCR4 Full
>>
>> This is getting less friendly all the time, and it was never very
>> friendly to start with. We have the strings which represent these link
>> modes in the table used for dumping caps. How about allowing the user
>> to list a comma separate list of modes.
>>
>> ethtool -s lan42 advertise 100000baseKR2/Full,100000baseSR2/Full,100000baseCR2/Full
> 
> In my preliminary netlink patchset, I'm adding support for e.g.
> 
>    ethtool -s eth0 advertise 100baseT/Half off 1000baseT/Full on
> 
> I'm not sure what would be more useful, if switching individual modes or
> listing the whole set. After all, we can also support both. But I fully
> agree that the numeric bitmaps are already too inconvenient.
> 
>>> +			adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
>>> +				  ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
>>> +				  ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
>>> +				  ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
>>> +				  ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
>>
>> Maybe i'm wrong, but this looks odd.
>>
>> enum ethtool_link_mode_bit_indices {
>>          ETHTOOL_LINK_MODE_10baseT_Half_BIT      = 0,
>>          ETHTOOL_LINK_MODE_10baseT_Full_BIT      = 1,
>>          ETHTOOL_LINK_MODE_100baseT_Half_BIT     = 2,
>>          ETHTOOL_LINK_MODE_100baseT_Full_BIT     = 3,
>>          ETHTOOL_LINK_MODE_1000baseT_Half_BIT    = 4,
>>          ETHTOOL_LINK_MODE_1000baseT_Full_BIT    = 5,
>>
>> These are numbers, not bitmasks, so & them together does not look
>> correct.
> 
> Yes, this is wrong. Even if bit masks were used, the operator should be
> "|" but here adv_bit is interpreted as an index, not mask. It's obvious
> this part of the code was designed when speed and duplex identified the
> mode uniquely which is no longer the case. (Which is probably also why
> there are no branches for speeds over 10G.)
> 
> And there is another problem:
> 
> +		else if (speed_wanted == SPEED_20000 &&
> +			 duplex_wanted == DUPLEX_FULL)
> +			adv_bit = ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT &
> +				  ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT &
> +				  ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT &
> +				  ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT &
> +				  ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT;
> 
> The test is for SPEED_20000 but then 200G modes are added.
> 
> Michal
> 
Thanks Michal and Andrew for your comments - I will fix them in the next 
version.
The code section (setting advertisement of 200G) will be removed 
completely, advertisement of 200G will be set to the supported 
link-modes that is handled below.
In addition I will make sure ethtool-copy.h is synced with current 
kernel version without additions.
As for the man page, I agree with you completely - I thought of 
replacing the LONG hex values with BIT(x) but since this can not be 
applied in the command line - I didn't implement it.

Aya

^ permalink raw reply

* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Ard Biesheuvel @ 2019-02-25 12:45 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
	Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
	Xinming Hu, Devicetree List, <netdev@vger.kernel.org>,
	<linux-wireless@vger.kernel.org>, Linux Kernel Mailing List,
	linux-rockchip, David S. Miller, linux-arm-kernel
In-Reply-To: <20190224140426.3267-1-marc.zyngier@arm.com>

On Sun, 24 Feb 2019 at 15:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> For quite some time, I wondered why the PCI mwifiex device built in my
> Chromebook was unable to use the good old legacy interrupts. But as MSIs
> were working fine, I never really bothered investigating. I finally had a
> look, and the result isn't very pretty.
>
> On this machine (rk3399-based kevin), the wake-up interrupt is described as
> such:
>
> &pci_rootport {
>         mvl_wifi: wifi@0,0 {
>                 compatible = "pci1b4b,2b42";
>                 reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>                        0x83010000 0x0 0x00100000 0x0 0x00100000>;
>                 interrupt-parent = <&gpio0>;
>                 interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&wlan_host_wake_l>;
>                 wakeup-source;
>         };
> };
>
> Note how the interrupt is part of the properties directly attached to the
> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
> altogether (Yay for the broken design!). This is in total violation of the
> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
> specifiers describe the PCI device interrupts, and must obey the
> INT-{A,B,C,D} mapping. Oops!
>

The mapping of legacy PCIe INTx interrupts onto wired system
interrupts is a property of the PCIe host controller, not of a
particular PCIe device. So I would argue that the code is broken here
as well: it should never attempt to interpret 'interrupt' properties
at the PCI device level as having any bearing on how legacy interrupts
are routed.

> The net effect of the above is that Linux tries to do something vaguely
> sensible, and uses the same interrupt for both the wake-up widget and the
> PCI device. This doesn't work for two reasons: (1) the wake-up widget grabs
> the interrupt in exclusive mode, and (2) the PCI interrupt is still routed
> to the RC, leading to a screaming interrupt. This simply cannot work.
>
> To sort out this mess, we need to lift the confusion between the two
> interrupts. This is done by extending the DT binding to allow the wake-up
> interrupt to be described in a 'wake-up' subnode, sidestepping the issue
> completely. On my Chromebook, it now looks like this:
>
> &pci_rootport {
>         mvl_wifi: wifi@0,0 {
>                 compatible = "pci1b4b,2b42";
>                 reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>                        0x83010000 0x0 0x00100000 0x0 0x00100000>;
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&wlan_host_wake_l>;
>                 wake-up {
>                         interrupt-parent = <&gpio0>;
>                         interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
>                         wakeup-source;
>                 };
>         };
> };
>
> The driver is then updated to look for this subnode first, and fallback to
> the original, broken behaviour (spitting out a warning in the offending
> configuration).
>
> For good measure, there are two additional patches:
>
> - The wake-up interrupt requesting is horribly racy, and could lead to
>   unpredictable behaviours. Let's fix that properly.
>
> - A final patch implementing the above transformation for the whole
>   RK3399-based Chromebook range, which all use the same broken
>   configuration.
>
> With all that, I finally have PCI legacy interrupts working with the mwifiex
> driver on my Chromebook.
>
> [1] http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
>
> Marc Zyngier (4):
>   dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a
>     separate node
>   mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists
>   mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling
>     it too late
>   arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own
>     subnode
>
>  .../bindings/net/wireless/marvell-8xxx.txt    | 23 ++++++++++++++++++-
>  .../dts/rockchip/rk3399-gru-chromebook.dtsi   |  8 ++++---
>  drivers/net/wireless/marvell/mwifiex/main.c   | 13 +++++++++--
>  3 files changed, 38 insertions(+), 6 deletions(-)
>
> --
> 2.20.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Neil Horman @ 2019-02-25 12:46 UTC (permalink / raw)
  To: Xin Long
  Cc: linux-kernel, network dev, linux-sctp, davem,
	Marcelo Ricardo Leitner
In-Reply-To: <2aee76766861ac9e46b7acf123d5d6e8f2adf8e0.1551093937.git.lucien.xin@gmail.com>

On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> sctp_hdr(skb) only works when skb->transport_header is set properly.
> 
> But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> 
> skb->transport_header is not guaranteed to be right value for sctp.
> It will cause to fail to check the checksum for sctp packets.
> 
> So fix it by using offset, which is always right in all places.
> 
> Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/net/sctp/checksum.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> index 32ee65a..1c6e6c0 100644
> --- a/include/net/sctp/checksum.h
> +++ b/include/net/sctp/checksum.h
> @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
>  static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
>  					unsigned int offset)
>  {
> -	struct sctphdr *sh = sctp_hdr(skb);
> +	struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
>  	const struct skb_checksum_ops ops = {
>  		.update  = sctp_csum_update,
>  		.combine = sctp_csum_combine,
> -- 
> 2.1.0
> 
> 
Shouldn't you use skb_set_transport_header and skb_transport_header here?

Neil


^ permalink raw reply

* Re: [PATCH net v2 2/2] selftests: pmtu: add explicit tests for PMTU exceptions cleanup
From: Stefano Brivio @ 2019-02-25 12:56 UTC (permalink / raw)
  To: Sabrina Dubroca, Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <20190225123330.GA10994@bistromath.localdomain>

On Mon, 25 Feb 2019 13:33:30 +0100
Sabrina Dubroca <sd@queasysnail.net> wrote:

> 2019-02-25, 12:13:46 +0100, Paolo Abeni wrote:
> > +	if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then  
> 
> That doesn't work. "ip link del" is stuck in a way that timeout can't
> terminate it, so this is still going to hang. Did you actually test
> this? :/

Indeed, upon actual testing, this hangs and the error is not reported.

> > +		err "  can't delete veth device in a timely
> > manner, PMTU dst likely leaked"
> > +		return 1
> > +	fi
> > +	return 0
> > +}  

You can use a subshell here, something like:

	${ns_a} ip link del dev veth_A-R1 &
	iplink_pid=$!
	sleep 1
	if [ "$(cat /proc/${iplink_pid}/cmdline 2>/dev/null | tr -d '\0')" = "iplinkdeldevveth_A-R1" ]; then
		err "  can't delete veth device in a timely manner, PMTU dst likely leaked"
		return 1
	fi
}

and now that I tried to run as a single test and couldn't find it, I
think it would also be worth it to rename the tests to something more
sensible.

Right now you have test_pmtu_ipv6_exception_cleanup() calling
test_cleanup_...(), I think it's confusing. Just call the test
test_cleanup_ipv6_exception(), it doesn't have so much to do with PMTU
anyway.

-- 
Stefano

^ permalink raw reply

* Re: [PATCH] net: dsa: Inherit dev addr from master
From: xiaofeis @ 2019-02-25 13:13 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
	Vivien Didelot, Florian Fainelli, Niklas Cassel, netdev
In-Reply-To: <20190222143003.GL5653@lunn.ch>

On 2019-02-22 22:30, Andrew Lunn wrote:
> On Fri, Feb 22, 2019 at 06:26:54PM +0530, Vinod Koul wrote:
>> From: Xiaofei Shen <xiaofeis@codeaurora.org>
>> 
>> When we create slave netdevice, the dev addr is inherited from master
>> but the master dev addr maybe NULL at that time, so inherit it again
>> while opening the slave.
> 
> Hi Xiaofei, Vinod
> 
> Inheriting it twice seems a bit clumsy. Can the first inherit be
> removed? Can you change your MAC driver to set its MAC address in
> probe?
> 
> 	Andrew
Hi Andrew

Some mac driver set its MAC address in probe while some MAC driver do it 
in the open function in the kernel.


This patch is checking whether the address is valid firstly, if it 
already inherits a valid mac address, it will not inherit again.

I think we can remove the first inherit, but if keep it, we can see 
consistent address output by ifconfig -a before open master and slave 
interface.

Xiaofeis


^ permalink raw reply

* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Xin Long @ 2019-02-25 13:20 UTC (permalink / raw)
  To: Neil Horman; +Cc: LKML, network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <20190225124614.GB30699@hmswarspite.think-freely.org>

On Mon, Feb 25, 2019 at 8:47 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> > sctp_hdr(skb) only works when skb->transport_header is set properly.
> >
> > But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> >
> > skb->transport_header is not guaranteed to be right value for sctp.
> > It will cause to fail to check the checksum for sctp packets.
> >
> > So fix it by using offset, which is always right in all places.
> >
> > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> > Reported-by: Li Shuang <shuali@redhat.com>
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  include/net/sctp/checksum.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> > index 32ee65a..1c6e6c0 100644
> > --- a/include/net/sctp/checksum.h
> > +++ b/include/net/sctp/checksum.h
> > @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> >  static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> >                                       unsigned int offset)
> >  {
> > -     struct sctphdr *sh = sctp_hdr(skb);
> > +     struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> >       const struct skb_checksum_ops ops = {
> >               .update  = sctp_csum_update,
> >               .combine = sctp_csum_combine,
> > --
> > 2.1.0
> >
> >
> Shouldn't you use skb_set_transport_header and skb_transport_header here?
you mean:
skb_set_transport_header(skb, offset);
sh = sctp_hdr(skb);
?

There's no place counting on here to set transport_header.
It will be a kinda redundant job, yet skb is 'const'.

^ permalink raw reply

* Re: [PATCH] net: dsa: Inherit dev addr from master
From: Andrew Lunn @ 2019-02-25 13:21 UTC (permalink / raw)
  To: xiaofeis
  Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
	Vivien Didelot, Florian Fainelli, Niklas Cassel, netdev
In-Reply-To: <441ae81a814498876e4cfed6fd5b7bcb@codeaurora.org>

> I think we can remove the first inherit, but if keep it, we can see
> consistent address output by ifconfig -a before open master and slave
> interface.

True

But it also means we see inconsistent MAC addresses on the master. The
MAC address changes on open.

Maybe it would be better to change your master so it sets its MAC
address during probe? Everything is then consistent all the time.

	Andrew

^ permalink raw reply

* Re: [PATCH] net: dsa: read mac address from DT for slave device
From: xiaofeis @ 2019-02-25 13:28 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
	Andrew Lunn, Vivien Didelot, Niklas Cassel, netdev
In-Reply-To: <9baf097f-37ea-2bef-92dd-25a7040a16ed@gmail.com>

Hi Florian

We have two slave DSA interfaces, wan0 and lan0, one is for wan port,
and the other is for lan port. Customer has it's mac address pool, they 
want
to assign the mac address from the pool on wan0, lan0, and other 
interfaces like
wifi, bt. Coreboot/uboot will populate it to the DTS node, so the driver 
can
get it from it's node. For DSA slave interface, it already has it's own 
DTS node, it's
easy to just add one porperty "local-mac-address" there for the usage in 
DSA driver.

If not use DSA framework, normally we will use eth0.x and eth0.y for wan 
and lan.
On this case, customer usually also assign the MAC address on these 
logical interface
from it's pool.

On 2019-02-22 23:43, Florian Fainelli wrote:
> On 2/22/19 4:58 AM, Vinod Koul wrote:
>> From: Xiaofei Shen <xiaofeis@codeaurora.org>
>> 
>> Before creating a slave netdevice, get the mac address from DTS and
>> apply in case it is valid.
> 
> Can you explain your use case in details?
> 
> Assigning a MAC address to a network device that represents a switch
> port does not quite make sense in general. The switch port is really
> representing one end of a pipe, so one side you have stations and on 
> the
> other side, you have the CPU/management Ethernet MAC controller's MAC
> address which constitutes a station as well. The DSA slave network
> devices are just software constructs meant to steer traffic towards
> specific ports of the switch, but they are all from the perpsective of
> traffic reaching the CPU Port in the first place, therefore traffic 
> that
> is generally a known unicast Ethernet frame with the CPU's MAC address
> as MAC DA (and of course all types of unknown MC, management traffic 
> etc.)
> 
> By default, DSA switch need to come up in a configuration where all
> ports (except CPU/management) must be strictly separate from every 
> other
> port such that we can achieve what a standalone Ethernet NIC would do.
> This works because all ports are isolated from one another, so there is
> no cross talk and so having the same MAC address (the one from the CPU)
> on the DSA slave network devices just works, each port is a separate
> broadcast domain.
> 
> Once you start bridging one or ore ports, the bridge root port will 
> have
> a MAC address, most likely the one the CPU/management Ethernet MAC, but
> similarly, this is not an issue and that's exactly how a software 
> bridge
> would work as well.
> 
>> 
>> Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>> ---
>>  include/net/dsa.h | 1 +
>>  net/dsa/dsa2.c    | 1 +
>>  net/dsa/slave.c   | 5 ++++-
>>  3 files changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index b3eefe8e18fd..aa24ce756679 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -198,6 +198,7 @@ struct dsa_port {
>>  	unsigned int		index;
>>  	const char		*name;
>>  	const struct dsa_port	*cpu_dp;
>> +	const char		*mac;
>>  	struct device_node	*dn;
>>  	unsigned int		ageing_time;
>>  	u8			stp_state;
>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>> index a1917025e155..afb7d9fa42f6 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -261,6 +261,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>>  	int err = 0;
>> 
>>  	memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>> +	dp->mac = of_get_mac_address(dp->dn);
>> 
>>  	if (dp->type != DSA_PORT_TYPE_UNUSED)
>>  		err = devlink_port_register(ds->devlink, &dp->devlink_port,
>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>> index a3fcc1d01615..8e64c4e947c6 100644
>> --- a/net/dsa/slave.c
>> +++ b/net/dsa/slave.c
>> @@ -1308,7 +1308,10 @@ int dsa_slave_create(struct dsa_port *port)
>>  	slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
>>  	slave_dev->hw_features |= NETIF_F_HW_TC;
>>  	slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>> -	eth_hw_addr_inherit(slave_dev, master);
>> +	if (port->mac && is_valid_ether_addr(port->mac))
>> +		ether_addr_copy(slave_dev->dev_addr, port->mac);
>> +	else
>> +		eth_hw_addr_inherit(slave_dev, master);
>>  	slave_dev->priv_flags |= IFF_NO_QUEUE;
>>  	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
>>  	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
>> 

^ permalink raw reply

* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Masami Hiramatsu @ 2019-02-25 13:36 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jann Horn, Nadav Amit, Alexei Starovoitov, Steven Rostedt,
	Linus Torvalds, Masami Hiramatsu, Linux List Kernel Mailing,
	Ingo Molnar, Andrew Morton, Changbin Du, Kees Cook,
	Daniel Borkmann, Network Development, bpf@vger.kernel.org,
	Rick Edgecombe, Dave Hansen, Peter Zijlstra (Intel), Igor Stoppa
In-Reply-To: <CALCETrU2V5KBA97g3O-yHiUu1acmM_K9b2a5ATmKSqGwdd-+Dw@mail.gmail.com>

On Fri, 22 Feb 2019 15:59:30 -0800
Andy Lutomirski <luto@kernel.org> wrote:

> On Fri, Feb 22, 2019 at 3:02 PM Jann Horn <jannh@google.com> wrote:
> >
> > On Fri, Feb 22, 2019 at 11:39 PM Nadav Amit <namit@vmware.com> wrote:
> > > > On Feb 22, 2019, at 2:21 PM, Nadav Amit <namit@vmware.com> wrote:
> > > >
> > > >> On Feb 22, 2019, at 2:17 PM, Jann Horn <jannh@google.com> wrote:
> > > >>
> > > >> On Fri, Feb 22, 2019 at 11:08 PM Nadav Amit <namit@vmware.com> wrote:
> > > >>>> On Feb 22, 2019, at 1:43 PM, Jann Horn <jannh@google.com> wrote:
> > > >>>>
> > > >>>> (adding some people from the text_poke series to the thread, removing stable@)
> > > >>>>
> > > >>>> On Fri, Feb 22, 2019 at 8:55 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > > >>>>>> On Feb 22, 2019, at 11:34 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > >>>>>>> On Fri, Feb 22, 2019 at 02:30:26PM -0500, Steven Rostedt wrote:
> > > >>>>>>> On Fri, 22 Feb 2019 11:27:05 -0800
> > > >>>>>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > >>>>>>>
> > > >>>>>>>>> On Fri, Feb 22, 2019 at 09:43:14AM -0800, Linus Torvalds wrote:
> > > >>>>>>>>>
> > > >>>>>>>>> Then we should still probably fix up "__probe_kernel_read()" to not
> > > >>>>>>>>> allow user accesses. The easiest way to do that is actually likely to
> > > >>>>>>>>> use the "unsafe_get_user()" functions *without* doing a
> > > >>>>>>>>> uaccess_begin(), which will mean that modern CPU's will simply fault
> > > >>>>>>>>> on a kernel access to user space.
> > > >>>>>>>>
> > > >>>>>>>> On bpf side the bpf_probe_read() helper just calls probe_kernel_read()
> > > >>>>>>>> and users pass both user and kernel addresses into it and expect
> > > >>>>>>>> that the helper will actually try to read from that address.
> > > >>>>>>>>
> > > >>>>>>>> If __probe_kernel_read will suddenly start failing on all user addresses
> > > >>>>>>>> it will break the expectations.
> > > >>>>>>>> How do we solve it in bpf_probe_read?
> > > >>>>>>>> Call probe_kernel_read and if that fails call unsafe_get_user byte-by-byte
> > > >>>>>>>> in the loop?
> > > >>>>>>>> That's doable, but people already complain that bpf_probe_read() is slow
> > > >>>>>>>> and shows up in their perf report.
> > > >>>>>>>
> > > >>>>>>> We're changing kprobes to add a specific flag to say that we want to
> > > >>>>>>> differentiate between kernel or user reads. Can this be done with
> > > >>>>>>> bpf_probe_read()? If it's showing up in perf report, I doubt a single
> > > >>>>>>
> > > >>>>>> so you're saying you will break existing kprobe scripts?
> > > >>>>>> I don't think it's a good idea.
> > > >>>>>> It's not acceptable to break bpf_probe_read uapi.
> > > >>>>>
> > > >>>>> If so, the uapi is wrong: a long-sized number does not reliably identify an address if you don’t separately know whether it’s a user or kernel address. s390x and 4G:4G x86_32 are the notable exceptions. I have lobbied for RISC-V and future x86_64 to join the crowd.  I don’t know whether I’ll win this fight, but the uapi will probably have to change for at least s390x.
> > > >>>>>
> > > >>>>> What to do about existing scripts is a different question.
> > > >>>>
> > > >>>> This lack of logical separation between user and kernel addresses
> > > >>>> might interact interestingly with the text_poke series, specifically
> > > >>>> "[PATCH v3 05/20] x86/alternative: Initialize temporary mm for
> > > >>>> patching" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-6-rick.p.edgecombe%40intel.com%2F&amp;data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&amp;sdata=HAbnDcrBne64JyPuVUMKmM7nQk67F%2BFvjuXEn8TmHeo%3D&amp;reserved=0)
> > > >>>> and "[PATCH v3 06/20] x86/alternative: Use temporary mm for text
> > > >>>> poking" (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20190221234451.17632-7-rick.p.edgecombe%40intel.com%2F&amp;data=02%7C01%7Cnamit%40vmware.com%7Cf2513009ef734ecd6b0d08d69913a5ae%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636864707020821793&amp;sdata=vNRIMKtFDy%2F3z5FlTwDiJY6VGEV%2FMHgQPTdFSFtCo4s%3D&amp;reserved=0),
> > > >>>> right? If someone manages to get a tracing BPF program to trigger in a
> > > >>>> task that has switched to the patching mm, could they use
> > > >>>> bpf_probe_write_user() - which uses probe_kernel_write() after
> > > >>>> checking that KERNEL_DS isn't active and that access_ok() passes - to
> > > >>>> overwrite kernel text that is mapped writable in the patching mm?
> > > >>>
> > > >>> Yes, this is a good point. I guess text_poke() should be defined with
> > > >>> “__kprobes” and open-code memcpy.
> > > >>>
> > > >>> Does it sound reasonable?
> > > >>
> > > >> Doesn't __text_poke() as implemented in the proposed patch use a
> > > >> couple other kernel functions, too? Like switch_mm_irqs_off() and
> > > >> pte_clear() (which can be a call into a separate function on paravirt
> > > >> kernels)?
> > > >
> > > > I will move the pte_clear() to be done after the poking mm was unloaded.
> > > > Give me a few minutes to send a sketch of what I think should be done.
> > >
> > > Err.. You are right, I don’t see an easy way of preventing a kprobe from
> > > being set on switch_mm_irqs_off(), and open-coding this monster is too ugly.
> > >
> > > The reasonable solution seems to me as taking all the relevant pieces of
> > > code (and data) that might be used during text-poking and encapsulating them, so they
> > > will be set in a memory area which cannot be kprobe'd. This can also be
> > > useful to write-protect data structures of code that calls text_poke(),
> > > e.g., static-keys. It can also protect data on that stack that is used
> > > during text_poke() from being overwritten from another core.
> > >
> > > This solution is somewhat similar to Igor Stoppa’s idea of using “enclaves”
> > > when doing write-rarely operations.
> > >
> > > Right now, I think that text_poke() will keep being susceptible to such
> > > an attack, unless you have a better suggestion.
> >
> > A relatively simple approach might be to teach BPF not to run kprobe
> > programs and such in contexts where current->mm isn't the active mm?
> > Maybe using nmi_uaccess_okay(), or something like that? It looks like
> > perf_callchain_user() also already uses that. Except that a lot of
> > this code is x86-specific...
> 
> This sounds like exactly the right solution.  If you're running from
> some unknown context (like NMI or tracing), then you should check
> nmi_uaccess_okay().  I think we should just promote that to be a
> non-arch-specific function (that returns true by default) and check it
> the relevant bpf_probe_xyz() functions.

This treat may also need for my work, like probe_user_read() we should
fail if nmi_uaccess_okay().

Thank you,

> 
> Alexei, does that seem reasonable?


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply

* Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Jamal Hadi Salim @ 2019-02-25 13:43 UTC (permalink / raw)
  To: Leslie Monis; +Cc: netdev, dave, vijaynsu, mysuryan
In-Reply-To: <20190225102051.12268-1-lesliemonis@gmail.com>

On 2019-02-25 5:20 a.m., Leslie Monis wrote:
> The current implementation of the PIE queuing discipline is according to the
> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> However, a lot of necessary modifications and enhancements have been proposed
> in RFC 8033, which have not yet been incorporated in the source code of Linux.
> This patch series helps in achieving the same.
> 
> Performance tests carried out using Flent [https://flent.org/]
> 

+Cc the authors of PIE to double check these values.
Please respond (it is how open source works!) and N/ACK

cheers,
jamal

^ permalink raw reply

* Re: [PATCH v2] xfrm: correctly check policy index in verify_newpolicy_info
From: Herbert Xu @ 2019-02-25 13:43 UTC (permalink / raw)
  To: Yue Haibing; +Cc: steffen.klassert, davem, linux-kernel, netdev
In-Reply-To: <20190225095600.26260-1-yuehaibing@huawei.com>

On Mon, Feb 25, 2019 at 05:56:00PM +0800, Yue Haibing wrote:
>
> the check. Then __xfrm_policy_unlink use the index to access array policy_count
> whose size is XFRM_POLICY_MAX * 2, triggering out of bounds access.

No it doesn't.  Even if it did the bug would be in __xfrm_policy_unlink
and not here.

Your patch makes no sense.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Jamal Hadi Salim @ 2019-02-25 13:47 UTC (permalink / raw)
  To: Leslie Monis; +Cc: netdev, dave
In-Reply-To: <e39403e3-4859-b7f1-b9cc-a8ff85864a9d@mojatatu.com>

On 2019-02-25 8:43 a.m., Jamal Hadi Salim wrote:
> On 2019-02-25 5:20 a.m., Leslie Monis wrote:
>> The current implementation of the PIE queuing discipline is according 
>> to the
>> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the 
>> paper
>> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
>> However, a lot of necessary modifications and enhancements have been 
>> proposed
>> in RFC 8033, which have not yet been incorporated in the source code 
>> of Linux.
>> This patch series helps in achieving the same.
>>
>> Performance tests carried out using Flent [https://flent.org/]
>>
> 
> +Cc the authors of PIE to double check these values.
> Please respond (it is how open source works!) and N/ACK

Great. Bouncing addresses.
D. Taht - can you look at this?

cheers,
jamal

^ permalink raw reply

* Re: Fw: [Bug 202651] New: Thunderbolt: Regression in atlantic.ko between 4.20 and 5.00-rc4 for Aquantia NIC 1d6a:87b1
From: Igor Russkikh @ 2019-02-25 14:03 UTC (permalink / raw)
  To: Stephen Hemminger, netdev@vger.kernel.org
  Cc: Dmitry Bogdanov, linux-kernel@vger.kernel.org
In-Reply-To: <20190223085055.04357494@shemminger-XPS-13-9360>

Hi Stephen, 

Thanks for heads up.

We are now looking into this, but the strange thing is atlantic driver had no valuable changes
between 4.20 and 5.0rc.

Also, from what user writes up in the ticket, the issue seems to be related with pci/thunderbolt
subsystem, not with the device driver itself: after couple of hot plugs/unplugs system hangs.

Will try to reproduce that ASAP on our hardware.

Regards,
  Igor


On 23.02.2019 19:50, Stephen Hemminger wrote:
> 
> 
> Begin forwarded message:
> 
> Date: Sat, 23 Feb 2019 09:01:29 +0000
> From: bugzilla-daemon@bugzilla.kernel.org
> To: stephen@networkplumber.org
> Subject: [Bug 202651] New: Regression in atlantic.ko between 4.20 and 5.00-rc4 for Aquantia NIC 1d6a:87b1
> 
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=202651
> 
>             Bug ID: 202651
>            Summary: Regression in atlantic.ko between 4.20 and 5.00-rc4
>                     for Aquantia NIC 1d6a:87b1
>            Product: Networking
>            Version: 2.5
>     Kernel Version: 5.00-rc4
>           Hardware: All
>                 OS: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: high
>           Priority: P1
>          Component: IPV4
>           Assignee: stephen@networkplumber.org
>           Reporter: nicholas.johnson@outlook.com.au
>         Regression: No
> 
> Hardware Details:
> ==============================================================================
> 
> VEN:DEV = 1d6a:87b1 "Aquantia Corp. AQC107 NBase-T/IEEE 802.3bz Ethernet
> Controller [AQtion]" Specifically, this is the newer AQC107S with "S"
> meaning "Secure" and the company could never elaborate on how it is more
> secure than the plain AQC107 (different VEN:DEV). This chip is inside
> the Promise SANLink3 N1 Thunderbolt 3 10GbE adapter.
> 
> 
> Severity:
> ==============================================================================
> 
> Marked high severity because it directly impacts user experience (cannot
> get the network to come up on the adapter).
> 
> 
> Back Story and Problem:
> ==============================================================================
> 
> In 5.00-rc4, on my Ubuntu desktop I have had extreme problems getting
> the connection up (borderline impossible). If I did manage (after
> messing with systemd *a lot*) then I got a dodgy connection with a lot
> of DNS resolution problems. At first I blamed it on my desktop which had
> a habit of corrupting the SSD partition tables on a regular basis, and
> hence I assumed that systemd was corrupted or something.
> 
> Recently, I tried on my laptop with Arch Linux and 5.00-rc4 and it had
> the exact same problem. It does not matter if you manually assign the 
> IPv4 address, or use DHCPv4.
> 
> Just now, on my laptop, I booted into 4.20.3-arch1-1-ARCH kernel from
> Arch Linux and it worked straight away.
> 
> 
> Expected behaviour:
> ==============================================================================
> 
> NetworkManager (or any tool) should bring the link up successfully.
> 
> 
> Reproduction:
> ==============================================================================
> 
> Should be reproducible with a 5.00-rc4 kernel and the 1d6a:87b1 NIC. I
> have no way of testing if other Aquantia devices are affected - I do not
> own any others. I am using Ubuntu and Arch (both the very latest) and I
> just cannot get NetworkManager to bring up the connection with 5.00-rc4
> (happens effortlessly with 4.20 kernel). If you want NetworkManager to
> manage the adapter, you may need to use the
> 10-globally-managed-devices.conf fix, depending on the version.
> 
> 
> Technical Notes:
> ==============================================================================
> 
> It is hard to tell, but my gut tells me that it is IPv4-related. That
> would explain why some sites could work. I have a native IPv6 service
> from my ISP. The "ifconfig" command shows IPv6 addresses assigned, but
> it is very difficult to get a DHCPv4-assigned address on the adapter.
> 
> A couple of times I did manage to get the adapter up in 5.00-rc4 with a
> lot of messing around, and the resulting internet connection was
> unreliable and unstable. DNS resolution seemed to be a big part of the
> problem.
> 
> All BARs are assigned correctly. Other than that, dmesg does not give
> anything interesting that I can tell, so if you want some debug logs
> from somewhere then please tell me what you want and how I should do it.
> 
> There are a fair few lines of diff between 4.20 and 5.00-rc4 so there is
> likely an oopsie in there.
> 
> 
> Always Remembering My Manners:
> ==============================================================================
> 
> Thank you for reading my bug report!
> 
> 
> Why Are Web Developers So Annoying?
> ==============================================================================
> 
> The preview removes all my newlines and nice formatting which I did so have fun
> trying to read this big blob of unformatted text.
> 

^ permalink raw reply

* Re: [PATCH net-next] net: sched: act_tunnel_key: fix metadata handling
From: Davide Caratti @ 2019-02-25 14:04 UTC (permalink / raw)
  To: Vlad Buslov, netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, wenxu, roid
In-Reply-To: <20190225122122.8128-1-vladbu@mellanox.com>

On Mon, 2019-02-25 at 14:21 +0200, Vlad Buslov wrote:
> Tunnel key action params->tcft_enc_metadata is only set when action is
> TCA_TUNNEL_KEY_ACT_SET. However, metadata pointer is incorrectly
> dereferenced during tunnel key init and release without verifying that
> action is if correct type, which causes NULL pointer dereference. Metadata
> tunnel dst_cache is also leaked on action overwrite.
> 
> Fix metadata handling:
> - Verify that metadata pointer is not NULL before dereferencing it in
>   tunnel_key_init error handling code.

hello Vlad,

thanks a lot for fixing this!

<...>
 
> @@ -384,10 +390,12 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
>  
>  release_dst_cache:
>  #ifdef CONFIG_DST_CACHE
> -	dst_cache_destroy(&metadata->u.tun_info.dst_cache);
> +	if (metadata)
> +		dst_cache_destroy(&metadata->u.tun_info.dst_cache);
>  #endif
>  release_tun_meta:
> -	dst_release(&metadata->dst);
> +	if (metadata)
> +		dst_release(&metadata->dst);

on Linux 'net' tree we don't have commit 41411e2fd6b8 ("net/sched:
act_tunnel_key: Add dst_cache support"), but still the above two lines can
avoid a NULL dereference in tunnel_key_init() error path, in the following
case:

* create an action with tunnel "set", with success
* replace the previous rule rule with tunnel "unset", and have a failure 
here (e.g. allocation of 'params_new').

At the cost of creating some conflicts during the merge, it would probably
be safer to split this commit into two parts, one targeting 'net' and one
targeting 'net-next', so that the first one can be proposed for stable
backports (and also I can rebase/retest my 'goto chain' series on top of
it :) )

WDYT?
-- 
davide



^ permalink raw reply


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