* [net-next 8/9] i40e: check for Tx timestamp timeouts during watchdog
From: Jeff Kirsher @ 2017-05-31 10:48 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170531104856.47929-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
The i40e driver has logic to handle only one Tx timestamp at a time,
using a state bit lock to avoid multiple requests at once.
It may be possible, if incredibly unlikely, that a Tx timestamp event is
requested but never completes. Since we use an interrupt scheme to
determine when the Tx timestamp occurred we would never clear the state
bit in this case.
Add an i40e_ptp_tx_hang() function similar to the already existing
i40e_ptp_rx_hang() function. This function runs in the watchdog routine
and makes sure we eventually recover from this case instead of
permanently disabling Tx timestamps.
Note: there is no currently known way to cause this without hacking the
driver code to force it.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 2 ++
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 30 +++++++++++++++++++++++++++++
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 1 +
4 files changed, 34 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index f4465afe1fe1..25bf336c5f38 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -502,6 +502,7 @@ struct i40e_pf {
struct ptp_clock *ptp_clock;
struct ptp_clock_info ptp_caps;
struct sk_buff *ptp_tx_skb;
+ unsigned long ptp_tx_start;
struct hwtstamp_config tstamp_config;
struct mutex tmreg_lock; /* Used to protect the SYSTIME registers. */
u64 ptp_base_adj;
@@ -957,6 +958,7 @@ bool i40e_dcb_need_reconfig(struct i40e_pf *pf,
struct i40e_dcbx_config *new_cfg);
#endif /* CONFIG_I40E_DCB */
void i40e_ptp_rx_hang(struct i40e_pf *pf);
+void i40e_ptp_tx_hang(struct i40e_pf *pf);
void i40e_ptp_tx_hwtstamp(struct i40e_pf *pf);
void i40e_ptp_rx_hwtstamp(struct i40e_pf *pf, struct sk_buff *skb, u8 index);
void i40e_ptp_set_increment(struct i40e_pf *pf);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index c019dec988e3..e4eb97832413 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6373,6 +6373,7 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
}
i40e_ptp_rx_hang(pf);
+ i40e_ptp_tx_hang(pf);
}
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index cb81e16d0874..1a0be835fa06 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -328,6 +328,36 @@ void i40e_ptp_rx_hang(struct i40e_pf *pf)
}
/**
+ * i40e_ptp_tx_hang - Detect error case when Tx timestamp register is hung
+ * @pf: The PF private data structure
+ *
+ * This watchdog task is run periodically to make sure that we clear the Tx
+ * timestamp logic if we don't obtain a timestamp in a reasonable amount of
+ * time. It is unexpected in the normal case but if it occurs it results in
+ * permanently prevent timestamps of future packets
+ **/
+void i40e_ptp_tx_hang(struct i40e_pf *pf)
+{
+ if (!(pf->flags & I40E_FLAG_PTP) || !pf->ptp_tx)
+ return;
+
+ /* Nothing to do if we're not already waiting for a timestamp */
+ if (!test_bit(__I40E_PTP_TX_IN_PROGRESS, pf->state))
+ return;
+
+ /* We already have a handler routine which is run when we are notified
+ * of a Tx timestamp in the hardware. If we don't get an interrupt
+ * within a second it is reasonable to assume that we never will.
+ */
+ if (time_is_before_jiffies(pf->ptp_tx_start + HZ)) {
+ dev_kfree_skb_any(pf->ptp_tx_skb);
+ pf->ptp_tx_skb = NULL;
+ clear_bit_unlock(__I40E_PTP_TX_IN_PROGRESS, pf->state);
+ pf->tx_hwtstamp_timeouts++;
+ }
+}
+
+/**
* i40e_ptp_tx_hwtstamp - Utility function which returns the Tx timestamp
* @pf: Board private structure
*
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c69ee4b0cfe2..c2e9013d05eb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2628,6 +2628,7 @@ static int i40e_tsyn(struct i40e_ring *tx_ring, struct sk_buff *skb,
if (pf->ptp_tx &&
!test_and_set_bit_lock(__I40E_PTP_TX_IN_PROGRESS, pf->state)) {
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
+ pf->ptp_tx_start = jiffies;
pf->ptp_tx_skb = skb_get(skb);
} else {
pf->tx_hwtstamp_skipped++;
--
2.12.2
^ permalink raw reply related
* [net-next 7/9] i40e: use pf data structure directly in i40e_ptp_rx_hang
From: Jeff Kirsher @ 2017-05-31 10:48 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <20170531104856.47929-1-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
There's no reason to pass a *vsi pointer if we already have the *pf
pointer in the only location where we call this function. Lets update
the signature and directly pass the *pf data structure pointer.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ptp.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index aa46ae016539..f4465afe1fe1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -956,7 +956,7 @@ bool i40e_dcb_need_reconfig(struct i40e_pf *pf,
struct i40e_dcbx_config *old_cfg,
struct i40e_dcbx_config *new_cfg);
#endif /* CONFIG_I40E_DCB */
-void i40e_ptp_rx_hang(struct i40e_vsi *vsi);
+void i40e_ptp_rx_hang(struct i40e_pf *pf);
void i40e_ptp_tx_hwtstamp(struct i40e_pf *pf);
void i40e_ptp_rx_hwtstamp(struct i40e_pf *pf, struct sk_buff *skb, u8 index);
void i40e_ptp_set_increment(struct i40e_pf *pf);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d5c9c9e06ff5..c019dec988e3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6372,7 +6372,7 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
i40e_update_veb_stats(pf->veb[i]);
}
- i40e_ptp_rx_hang(pf->vsi[pf->lan_vsi]);
+ i40e_ptp_rx_hang(pf);
}
/**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index aead71a92a60..cb81e16d0874 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -269,6 +269,7 @@ static u32 i40e_ptp_get_rx_events(struct i40e_pf *pf)
/**
* i40e_ptp_rx_hang - Detect error case when Rx timestamp registers are hung
+ * @pf: The PF private data structure
* @vsi: The VSI with the rings relevant to 1588
*
* This watchdog task is scheduled to detect error case where hardware has
@@ -276,9 +277,8 @@ static u32 i40e_ptp_get_rx_events(struct i40e_pf *pf)
* particular error is rare but leaves the device in a state unable to timestamp
* any future packets.
**/
-void i40e_ptp_rx_hang(struct i40e_vsi *vsi)
+void i40e_ptp_rx_hang(struct i40e_pf *pf)
{
- struct i40e_pf *pf = vsi->back;
struct i40e_hw *hw = &pf->hw;
unsigned int i, cleared = 0;
--
2.12.2
^ permalink raw reply related
* [net-next 9/9] i40e: Check for memory allocation failure
From: Jeff Kirsher @ 2017-05-31 10:48 UTC (permalink / raw)
To: davem; +Cc: Christophe Jaillet, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <20170531104856.47929-1-jeffrey.t.kirsher@intel.com>
From: Christophe Jaillet <christophe.jaillet@wanadoo.fr>
If 'kzalloc' fails, a NULL pointer will be dereferenced. Return -ENOMEM
instead.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_client.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index c3b81a97558e..088b4a43bd2a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -595,6 +595,8 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
size = sizeof(struct i40e_qvlist_info) +
(sizeof(struct i40e_qv_info) * (qvlist_info->num_vectors - 1));
ldev->qvlist_info = kzalloc(size, GFP_KERNEL);
+ if (!ldev->qvlist_info)
+ return -ENOMEM;
ldev->qvlist_info->num_vectors = qvlist_info->num_vectors;
for (i = 0; i < qvlist_info->num_vectors; i++) {
--
2.12.2
^ permalink raw reply related
* Re: [PATCH net-next] powerpc: use asm-generic/socket.h as much as possible
From: Stephen Rothwell @ 2017-05-31 11:06 UTC (permalink / raw)
To: Michael Ellerman
Cc: David Miller, Networking, Benjamin Herrenschmidt, PowerPC,
Arnd Bergmann
In-Reply-To: <878tld49ic.fsf@concordia.ellerman.id.au>
Hi Michael,
On Wed, 31 May 2017 20:15:55 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Stephen Rothwell <sfr@canb.auug.org.au> writes:
>
> > asm-generic/socket.h already has an exception for the differences that
> > powerpc needs, so just include it after defining the differences.
> >
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > ---
> > arch/powerpc/include/uapi/asm/socket.h | 92 +---------------------------------
> > 1 file changed, 1 insertion(+), 91 deletions(-)
> >
> > Build tested using powerpc allyesconfig, pseries_le_defconfig, 32 bit
> > and 64 bit allnoconfig and ppc44x_defconfig builds.
>
> Did you boot it and test that userspace was happy doing sockety things?
No, sorry.
The patch was done by inspection, but it is pretty obvious ... here is
the diff between arch/powerpc/include/uapi/asm/socket.h and
include/uapi/asm-generic/socket.h before the patch:
--- arch/powerpc/include/uapi/asm/socket.h 2017-05-31 20:56:54.940473709 +1000
+++ include/uapi/asm-generic/socket.h 2017-05-31 10:04:16.716445463 +1000
@@ -1,12 +1,5 @@
-#ifndef _ASM_POWERPC_SOCKET_H
-#define _ASM_POWERPC_SOCKET_H
-
-/*
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- */
+#ifndef __ASM_GENERIC_SOCKET_H
+#define __ASM_GENERIC_SOCKET_H
#include <asm/sockios.h>
@@ -30,12 +23,14 @@
#define SO_LINGER 13
#define SO_BSDCOMPAT 14
#define SO_REUSEPORT 15
-#define SO_RCVLOWAT 16
-#define SO_SNDLOWAT 17
-#define SO_RCVTIMEO 18
-#define SO_SNDTIMEO 19
-#define SO_PASSCRED 20
-#define SO_PEERCRED 21
+#ifndef SO_PASSCRED /* powerpc only differs in these */
+#define SO_PASSCRED 16
+#define SO_PEERCRED 17
+#define SO_RCVLOWAT 18
+#define SO_SNDLOWAT 19
+#define SO_RCVTIMEO 20
+#define SO_SNDTIMEO 21
+#endif
/* Security levels - as per NRL IPv6 - don't actually do anything */
#define SO_SECURITY_AUTHENTICATION 22
@@ -71,7 +66,7 @@
#define SO_RXQ_OVFL 40
#define SO_WIFI_STATUS 41
-#define SCM_WIFI_STATUS SO_WIFI_STATUS
+#define SCM_WIFI_STATUS SO_WIFI_STATUS
#define SO_PEEK_OFF 42
/* Instruct lower device to use last 4-bytes of skb data as FCS */
@@ -107,4 +102,4 @@
#define SCM_TIMESTAMPING_PKTINFO 58
-#endif /* _ASM_POWERPC_SOCKET_H */
+#endif /* __ASM_GENERIC_SOCKET_H */
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: [PATCH net] mpls: fix clearing of dead nh_flags on link up
From: David Ahern @ 2017-05-31 11:18 UTC (permalink / raw)
To: Roopa Prabhu, davem; +Cc: netdev
In-Reply-To: <1496213436-33961-1-git-send-email-roopa@cumulusnetworks.com>
On 5/31/17 12:50 AM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> recent fixes to use WRITE_ONCE for nh_flags on link up,
> accidently ended up leaving the deadflags on a nh. This patch
> fixes the WRITE_ONCE to use freshly evaluated nh_flags.
>
> Fixes: 39eb8cd17588 ("net: mpls: rt_nhn_alive and nh_flags should be accessed using READ_ONCE")
> Reported-by: Satish Ashok <sashok@cumulusnetworks.com>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> net/mpls/af_mpls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* [PATCH net] ipv6: xfrm: Handle errors reported by xfrm6_find_1stfragopt()
From: Ben Hutchings @ 2017-05-31 12:15 UTC (permalink / raw)
To: Craig Gallek, David S. Miller; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]
xfrm6_find_1stfragopt() may now return an error code and we must
not treat it as a length.
Fixes: 2423496af35d ("ipv6: Prevent overrun when parsing v6 header options")
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
Commits 2423496af35d "ipv6: Prevent overrun when parsing v6 header
options" and 7dd7eb9513bd "ipv6: Check ip6_find_1stfragopt() return
value properly." changed ip6_find_1stfragopt() to return negative
error codes and changed some of its callers to handle this.
However, there is also xfrm6_find_1stfragopt() which is a wrapper for
ip6_find_1stfragopt() and is called indirectly by xfrm6_ro_output()
and xfrm6_transport_output(). Neither of them check for errors.
This is totally untested. I think xfrm_type::hdr_offset deserves a
comment about its semantics, but I don't understand it well enogugh to
write that.
Ben.
net/ipv6/xfrm6_mode_ro.c | 2 ++
net/ipv6/xfrm6_mode_transport.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/net/ipv6/xfrm6_mode_ro.c b/net/ipv6/xfrm6_mode_ro.c
index 0e015906f9ca..07d36573f50b 100644
--- a/net/ipv6/xfrm6_mode_ro.c
+++ b/net/ipv6/xfrm6_mode_ro.c
@@ -47,6 +47,8 @@ static int xfrm6_ro_output(struct xfrm_state *x, struct sk_buff *skb)
iph = ipv6_hdr(skb);
hdr_len = x->type->hdr_offset(x, skb, &prevhdr);
+ if (hdr_len < 0)
+ return hdr_len;
skb_set_mac_header(skb, (prevhdr - x->props.header_len) - skb->data);
skb_set_network_header(skb, -x->props.header_len);
skb->transport_header = skb->network_header + hdr_len;
diff --git a/net/ipv6/xfrm6_mode_transport.c b/net/ipv6/xfrm6_mode_transport.c
index 7a92c0f31912..9ad07a91708e 100644
--- a/net/ipv6/xfrm6_mode_transport.c
+++ b/net/ipv6/xfrm6_mode_transport.c
@@ -30,6 +30,8 @@ static int xfrm6_transport_output(struct xfrm_state *x, struct sk_buff *skb)
skb_set_inner_transport_header(skb, skb_transport_offset(skb));
hdr_len = x->type->hdr_offset(x, skb, &prevhdr);
+ if (hdr_len < 0)
+ return hdr_len;
skb_set_mac_header(skb, (prevhdr - x->props.header_len) - skb->data);
skb_set_network_header(skb, -x->props.header_len);
skb->transport_header = skb->network_header + hdr_len;
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related
* Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed
From: Arend van Spriel @ 2017-05-31 12:15 UTC (permalink / raw)
To: Kalle Valo, Jia-Ju Bai
Cc: Larry.Finger, linux-wireless, b43-dev, netdev, linux-kernel
In-Reply-To: <877f0xnwyk.fsf@kamboji.qca.qualcomm.com>
On 31-05-17 12:26, Kalle Valo wrote:
> Jia-Ju Bai <baijiaju1990@163.com> writes:
>
>> The driver may sleep under a spin lock, and the function call path is:
>> b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave)
>> b43legacy_synchronize_irq
>> synchronize_irq --> may sleep
>>
>> To fix it, the lock is released before b43legacy_synchronize_irq, and the
>> lock is acquired again after this function.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
>> ---
>> drivers/net/wireless/broadcom/b43legacy/main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c b/drivers/net/wireless/broadcom/b43legacy/main.c
>> index f1e3dad..31ead21 100644
>> --- a/drivers/net/wireless/broadcom/b43legacy/main.c
>> +++ b/drivers/net/wireless/broadcom/b43legacy/main.c
>> @@ -2859,7 +2859,9 @@ static void b43legacy_op_bss_info_changed(struct ieee80211_hw *hw,
>> b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
>>
>> if (changed & BSS_CHANGED_BSSID) {
>> + spin_unlock_irqrestore(&wl->irq_lock, flags);
>> b43legacy_synchronize_irq(dev);
>> + spin_lock_irqsave(&wl->irq_lock, flags);
>
> To me this looks like a fragile workaround and not a real fix. You can
> easily add new race conditions with releasing the lock like this.
Hi Jia-Ju,
Agree with Kalle as I was about to say the same thing. You really need
to determine what is protected by the irq_lock. Here you are using the
lock because you are about to change wl->bssid a bit further down. Did
not check the entire function but it seems the lock perimeter is too wide.
Regards,
Arend
^ permalink raw reply
* Leak in ipv6_gso_segment()?
From: Ben Hutchings @ 2017-05-31 12:26 UTC (permalink / raw)
To: Craig Gallek, David S. Miller; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
If I'm not mistaken, ipv6_gso_segment() now leaks segs if
ip6_find_1stfragopt() fails. I'm not sure whether the fix would be as
simple as adding a kfree_skb(segs) or whether more complex cleanup is
needed.
Ben.
--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [patch iproute2 v2 repost 3/3] tc/actions: introduce support for goto chain action
From: Jiri Benc @ 2017-05-31 12:27 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jhs, xiyou.wangcong, dsa, edumazet, stephen,
daniel, alexander.h.duyck, simon.horman, mlxsw
In-Reply-To: <20170516172937.1391-3-jiri@resnulli.us>
On Tue, 16 May 2017 19:29:37 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Allow user to set control action "goto" with filter chain index as
> a parameter.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> include/linux/pkt_cls.h | 16 +++++++++++++++-
> man/man8/tc-ife.8 | 2 +-
> man/man8/tc-pedit.8 | 2 +-
> man/man8/tc-police.8 | 2 +-
> man/man8/tc-vlan.8 | 2 +-
Jiri, this *seriously* needs a proper documentation. I consider myself
to be well above average in understanding tc filters and actions, and as
you know I participated at least passively in some chats about the
chains. Yet I'm still very lost in semantics of chains. I cannot
imagine how an average user would be able to use this.
Some questions about the semantics:
- What chain(s) are processed by default? If I don't use goto_chain but
have multiple chains, will only chain 0 be processed, or more? In
what order, what are the rules? If I don't have a chain 0, what would
happen?
- What is the relation between priorities and chains? I expected that
chains are under the same priority but that's not the case in your
example. For a user, it's very unclear why there are priorities and
chains. They appear to do the same thing, why can't I just jump
between priorities?
Please, in addition to explaining here on the list, add detailed
explanation to the man pages.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH net-next] netlink: include netnsid only when netns differs.
From: Flavio Leitner @ 2017-05-31 12:28 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev
In-Reply-To: <1b83c01b-8fe3-8e9b-cae7-4064527c1c21@6wind.com>
On Wed, May 31, 2017 at 10:38:21AM +0200, Nicolas Dichtel wrote:
> Le 30/05/2017 à 23:33, Flavio Leitner a écrit :
> > Don't include netns id for notifications broadcasts when the
> > socket and the skb are in the same netns because it will be
> > an error which can't be distinguished from a peer netns failing
> > to allocate an id.
> I don't understand the problem. peernet2id() doesn't allocate ids, it only do a
> lookup. If you need an id for the current netns, you have to allocate one.
The issue is that if you query an interface on the same netns, the
error is returned, then we cannot tell if the iface is on the same
netns or if there was an error while allocating the ID and the
iface is on another netns.
> This patch changes the metadata exported to the userland and will break existing
> tools.
It should not break because it changes only for interfaces on
the same netns where there is no ID and that value wasn't
exported until recently.
--
Flavio
^ permalink raw reply
* Re: loosing netdevices with namespaces and unshare?
From: Harald Welte @ 2017-05-31 12:27 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpW2ARBTmGK8JhVXSFCaB1F-WPDKeDZwEbxMLf_Xi7aXQw@mail.gmail.com>
Hi Cong,
On Tue, May 30, 2017 at 04:18:17PM -0700, Cong Wang wrote:
> On Tue, May 30, 2017 at 3:07 PM, Harald Welte <laforge@gnumonks.org> wrote:
> > But, to the contrary, this doesn't happen. The unshare-created netns is
> > gone, but the netdevice did not get moved back to the root namespace
> > either. The only hack to get back to the "eth0" device is to unload the
> > driver and re-load it.
>
>
> Net namespace simply unregisters all netdevices inside when it is
> gone, no matter where they are from.
ah, ok. I missed that part. Is there a good piece of documentation on
netwokr namespaces that I should read?
> I am pretty sure you can move it back to root-ns if you want,
Yes, I can explicitly do that, but this of course doesn't work if e.g.
my [single] process in that namespace crashes due to some bug, OOM or
the like.
> it is a little tricky because you have to give the root-ns a name
> first.
It's actually not, as you can just identify the root-ns by pid 1, so
"ip link set $DEV netns 1" will move it back. As indicated, I'm worried
about the error paths.
> > What am I missing here? Is this the intended behavior?
>
> Yes it is.
thanks for your confirmation. Guess I have to get used to it.
> > Of course I know I could simply do something like "ip link set eth0
> > netns 1" from within the namespace before leaving. But what if the
> > process is not bash and the process exits abnormally? I'd consider
> > that explicit reassignment more like a hack than a proper solution...
>
> It doesn't make sense to move it back to where it is from, for example,
> what if you move a veth0 from netns1 to netns2 and netns1 is gone
> before netns2?
for virtual devices, I would agree. For physical devices, I think the
default behavior to unregister them is - from my of course very
subjective point of view - quite questionable.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* [PATCH] net: don't call strlen on non-terminated string in dev_set_alias()
From: Alexander Potapenko @ 2017-05-31 12:33 UTC (permalink / raw)
To: dvyukov, kcc, edumazet, davem; +Cc: linux-kernel, netdev
KMSAN reported a use of uninitialized memory in dev_set_alias(),
which was caused by calling strlcpy() (which in turn called strlen())
on the user-supplied non-terminated string.
Signed-off-by: Alexander Potapenko <glider@google.com>
---
For the record, here is the KMSAN report:
==================================================================
BUG: KMSAN: use of unitialized memory in strlcpy+0x8b/0x1b0
CPU: 0 PID: 1062 Comm: probe Not tainted 4.11.0-rc5+ #2763
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x143/0x1b0 lib/dump_stack.c:52
kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:1016
__kmsan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:491
strlen lib/string.c:484
strlcpy+0x8b/0x1b0 lib/string.c:144
dev_set_alias+0x295/0x360 net/core/dev.c:1255
do_setlink+0xfe5/0x5750 net/core/rtnetlink.c:2007
rtnl_setlink+0x469/0x4f0 net/core/rtnetlink.c:2249
rtnetlink_rcv_msg+0x5da/0xb40 net/core/rtnetlink.c:4107
netlink_rcv_skb+0x339/0x5a0 net/netlink/af_netlink.c:2339
rtnetlink_rcv+0x83/0xa0 net/core/rtnetlink.c:4113
netlink_unicast_kernel net/netlink/af_netlink.c:1272
netlink_unicast+0x13b7/0x1480 net/netlink/af_netlink.c:1298
netlink_sendmsg+0x10b8/0x10f0 net/netlink/af_netlink.c:1844
sock_sendmsg_nosec net/socket.c:633
sock_sendmsg net/socket.c:643
sock_write_iter+0x395/0x440 net/socket.c:857
call_write_iter ./include/linux/fs.h:1733
new_sync_write fs/read_write.c:497
__vfs_write+0x619/0x700 fs/read_write.c:510
vfs_write+0x47e/0x8a0 fs/read_write.c:558
SYSC_write+0x17e/0x2f0 fs/read_write.c:605
SyS_write+0x87/0xb0 fs/read_write.c:597
entry_SYSCALL_64_fastpath+0x13/0x94 arch/x86/entry/entry_64.S:204
RIP: 0033:0x4052f1
RSP: 002b:00007fde1ed05d80 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004052f1
RDX: 0000000000000026 RSI: 00000000006cf0c0 RDI: 0000000000000032
RBP: 00007fde1ed05da0 R08: 00007fde1ed06700 R09: 00007fde1ed06700
R10: 00007fde1ed069d0 R11: 0000000000000293 R12: 0000000000000000
R13: 0000000000000000 R14: 00007fde1ed069c0 R15: 00007fde1ed06700
origin: 00000000aec00057
save_stack_trace+0x59/0x60 arch/x86/kernel/stacktrace.c:59
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:352
kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:247
kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:260
slab_alloc_node mm/slub.c:2743
__kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4349
__kmalloc_reserve net/core/skbuff.c:138
__alloc_skb+0x2cd/0x740 net/core/skbuff.c:231
alloc_skb ./include/linux/skbuff.h:933
netlink_alloc_large_skb net/netlink/af_netlink.c:1144
netlink_sendmsg+0x934/0x10f0 net/netlink/af_netlink.c:1819
sock_sendmsg_nosec net/socket.c:633
sock_sendmsg net/socket.c:643
sock_write_iter+0x395/0x440 net/socket.c:857
call_write_iter ./include/linux/fs.h:1733
new_sync_write fs/read_write.c:497
__vfs_write+0x619/0x700 fs/read_write.c:510
vfs_write+0x47e/0x8a0 fs/read_write.c:558
SYSC_write+0x17e/0x2f0 fs/read_write.c:605
SyS_write+0x87/0xb0 fs/read_write.c:597
entry_SYSCALL_64_fastpath+0x13/0x94 arch/x86/entry/entry_64.S:204
==================================================================
and the reproducer:
==================================================================
#include <pthread.h>
int sock = -1;
char buf[] = "\x26\x00\x00\x00\x13\x00\x47\xf1\x07\x01\xc1\xb0"
"\x0e\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00"
"\x09\xef\x18\xff\xff\x00\xf1\x32\x05\x00\x14\x00"
"\x6e\x35";
void do_work(int arg) {
switch (arg) {
case 0:
sock = socket(0x10, 0x3, 0x0);
break;
case 1:
write(sock, buf, 0x26);
break;
}
}
void *thread_fn(void *arg) {
int actual_arg = (int)arg;
int iter = 10000, i;
for (i = 0; i < iter; i++) {
do_work(actual_arg);
usleep(100);
}
return NULL;
}
int main() {
pthread_t thr[4];
int i;
for (i = 0; i < 4; i++) {
pthread_create(&thr, NULL, thread_fn, (void*)(i % 2));
}
sleep(10);
return 0;
}
==================================================================
---
net/core/dev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index fca407b4a6ea..1a4ac1ba5dfd 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1254,7 +1254,10 @@ int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
return -ENOMEM;
dev->ifalias = new_ifalias;
- strlcpy(dev->ifalias, alias, len+1);
+ /* alias comes from the userspace and may not be zero-terminated.
+ */
+ memcpy(dev->ifalias, alias, len + 1);
+ dev->ifalias[len + 1] = 0;
return len;
}
--
2.13.0.219.gdb65acc882-goog
^ permalink raw reply related
* [PATCH v2] net: don't call strlen on non-terminated string in dev_set_alias()
From: Alexander Potapenko @ 2017-05-31 12:51 UTC (permalink / raw)
To: dvyukov, kcc, edumazet, davem; +Cc: linux-kernel, netdev
KMSAN reported a use of uninitialized memory in dev_set_alias(),
which was caused by calling strlcpy() (which in turn called strlen())
on the user-supplied non-terminated string.
Signed-off-by: Alexander Potapenko <glider@google.com>
---
v2: fixed an off-by-one error spotted by Dmitry Vyukov
For the record, here is the KMSAN report:
==================================================================
BUG: KMSAN: use of unitialized memory in strlcpy+0x8b/0x1b0
CPU: 0 PID: 1062 Comm: probe Not tainted 4.11.0-rc5+ #2763
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x143/0x1b0 lib/dump_stack.c:52
kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:1016
__kmsan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:491
strlen lib/string.c:484
strlcpy+0x8b/0x1b0 lib/string.c:144
dev_set_alias+0x295/0x360 net/core/dev.c:1255
do_setlink+0xfe5/0x5750 net/core/rtnetlink.c:2007
rtnl_setlink+0x469/0x4f0 net/core/rtnetlink.c:2249
rtnetlink_rcv_msg+0x5da/0xb40 net/core/rtnetlink.c:4107
netlink_rcv_skb+0x339/0x5a0 net/netlink/af_netlink.c:2339
rtnetlink_rcv+0x83/0xa0 net/core/rtnetlink.c:4113
netlink_unicast_kernel net/netlink/af_netlink.c:1272
netlink_unicast+0x13b7/0x1480 net/netlink/af_netlink.c:1298
netlink_sendmsg+0x10b8/0x10f0 net/netlink/af_netlink.c:1844
sock_sendmsg_nosec net/socket.c:633
sock_sendmsg net/socket.c:643
sock_write_iter+0x395/0x440 net/socket.c:857
call_write_iter ./include/linux/fs.h:1733
new_sync_write fs/read_write.c:497
__vfs_write+0x619/0x700 fs/read_write.c:510
vfs_write+0x47e/0x8a0 fs/read_write.c:558
SYSC_write+0x17e/0x2f0 fs/read_write.c:605
SyS_write+0x87/0xb0 fs/read_write.c:597
entry_SYSCALL_64_fastpath+0x13/0x94 arch/x86/entry/entry_64.S:204
RIP: 0033:0x4052f1
RSP: 002b:00007fde1ed05d80 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004052f1
RDX: 0000000000000026 RSI: 00000000006cf0c0 RDI: 0000000000000032
RBP: 00007fde1ed05da0 R08: 00007fde1ed06700 R09: 00007fde1ed06700
R10: 00007fde1ed069d0 R11: 0000000000000293 R12: 0000000000000000
R13: 0000000000000000 R14: 00007fde1ed069c0 R15: 00007fde1ed06700
origin: 00000000aec00057
save_stack_trace+0x59/0x60 arch/x86/kernel/stacktrace.c:59
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:352
kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:247
kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:260
slab_alloc_node mm/slub.c:2743
__kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4349
__kmalloc_reserve net/core/skbuff.c:138
__alloc_skb+0x2cd/0x740 net/core/skbuff.c:231
alloc_skb ./include/linux/skbuff.h:933
netlink_alloc_large_skb net/netlink/af_netlink.c:1144
netlink_sendmsg+0x934/0x10f0 net/netlink/af_netlink.c:1819
sock_sendmsg_nosec net/socket.c:633
sock_sendmsg net/socket.c:643
sock_write_iter+0x395/0x440 net/socket.c:857
call_write_iter ./include/linux/fs.h:1733
new_sync_write fs/read_write.c:497
__vfs_write+0x619/0x700 fs/read_write.c:510
vfs_write+0x47e/0x8a0 fs/read_write.c:558
SYSC_write+0x17e/0x2f0 fs/read_write.c:605
SyS_write+0x87/0xb0 fs/read_write.c:597
entry_SYSCALL_64_fastpath+0x13/0x94 arch/x86/entry/entry_64.S:204
==================================================================
and the reproducer:
==================================================================
#include <pthread.h>
int sock = -1;
char buf[] = "\x26\x00\x00\x00\x13\x00\x47\xf1\x07\x01\xc1\xb0"
"\x0e\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00"
"\x09\xef\x18\xff\xff\x00\xf1\x32\x05\x00\x14\x00"
"\x6e\x35";
void do_work(int arg) {
switch (arg) {
case 0:
sock = socket(0x10, 0x3, 0x0);
break;
case 1:
write(sock, buf, 0x26);
break;
}
}
void *thread_fn(void *arg) {
int actual_arg = (int)arg;
int iter = 10000, i;
for (i = 0; i < iter; i++) {
do_work(actual_arg);
usleep(100);
}
return NULL;
}
int main() {
pthread_t thr[4];
int i;
for (i = 0; i < 4; i++) {
pthread_create(&thr, NULL, thread_fn, (void*)(i % 2));
}
sleep(10);
return 0;
}
==================================================================
---
net/core/dev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index fca407b4a6ea..757069baffa9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1254,7 +1254,10 @@ int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
return -ENOMEM;
dev->ifalias = new_ifalias;
- strlcpy(dev->ifalias, alias, len+1);
+ /* alias comes from the userspace and may not be zero-terminated.
+ */
+ memcpy(dev->ifalias, alias, len);
+ dev->ifalias[len] = 0;
return len;
}
--
2.13.0.219.gdb65acc882-goog
^ permalink raw reply related
* [PATCH net] cxgb4: avoid enabling napi twice to the same queue
From: Ganesh Goudar @ 2017-05-31 12:56 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, Ganesh Goudar
Take uld mutex to avoid race between cxgb_up() and
cxgb4_register_uld() to enable napi for the same uld
queue.
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index b512149..77ed2f6 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -2196,10 +2196,14 @@ static int cxgb_up(struct adapter *adap)
if (err)
goto irq_err;
}
+
+ mutex_lock(&uld_mutex);
enable_rx(adap);
t4_sge_start(adap);
t4_intr_enable(adap);
adap->flags |= FULL_INIT_DONE;
+ mutex_unlock(&uld_mutex);
+
notify_ulds(adap, CXGB4_STATE_UP);
#if IS_ENABLED(CONFIG_IPV6)
update_clip(adap);
--
2.1.0
^ permalink raw reply related
* [PATCH net-next] cxgb4: fix incorrect cim_la output for T6
From: Ganesh Goudar @ 2017-05-31 13:40 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, Ganesh Goudar
take care of UpDbgLaRdPtr[0-3] restriction for T6.
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
v2:
- not using loop to increment UPDBGLARDPTR
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 9160c88..822c560 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -8312,7 +8312,16 @@ int t4_cim_read_la(struct adapter *adap, u32 *la_buf, unsigned int *wrptr)
ret = t4_cim_read(adap, UP_UP_DBG_LA_DATA_A, 1, &la_buf[i]);
if (ret)
break;
- idx = (idx + 1) & UPDBGLARDPTR_M;
+
+ /* Bits 0-3 of UpDbgLaRdPtr can be between 0000 to 1001 to
+ * identify the 32-bit portion of the full 312-bit data
+ */
+ if (is_t6(adap->params.chip) && (idx & 0xf) >= 9)
+ idx = (idx & 0xff0) + 0x10;
+ else
+ idx++;
+ /* address can't exceed 0xfff */
+ idx &= UPDBGLARDPTR_M;
}
restart:
if (cfg & UPDBGLAEN_F) {
--
2.1.0
^ permalink raw reply related
* Re: 'iw events' stops receiving events after a while on 4.9 + hacks
From: Ben Greear @ 2017-05-31 13:46 UTC (permalink / raw)
To: Bastian Bittorf, Johannes Berg; +Cc: netdev, linux-wireless@vger.kernel.org
In-Reply-To: <20170531081836.GC2849@medion.lan>
On 05/31/2017 01:18 AM, Bastian Bittorf wrote:
> * Johannes Berg <johannes@sipsolutions.net> [31.05.2017 10:09]:
>>> Is there any way to dump out the socket information if we reproduce
>>> the problem?
>>
>> I have no idea, sorry.
>>
>> If you or Bastian can tell me how to reproduce the problem, I can try
>> to investigate it.
>
> there was an interesting fix regarding the shell-builtin 'read' in
> busybox[1]. I will retest again and report if this changes anything.
>
> bye, bastian
>
> PS: @ben: are you also using 'iw event | while read -r LINE ...'?
I'm using a perl script to read the output, and not using busybox.
I have not seen the problem again, so it is not easy for me to reproduce.
If you reproduce it, maybe check 'strace' on the 'iw' process to see if it is
hung on writing output to the pipe or reading input? In my case, it appeared
to be hung reading input from netlink, input that never arrived.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH net-next] netlink: include netnsid only when netns differs.
From: Nicolas Dichtel @ 2017-05-31 13:48 UTC (permalink / raw)
To: Flavio Leitner; +Cc: netdev
In-Reply-To: <20170531122836.GV2673@x240.lan>
Le 31/05/2017 à 14:28, Flavio Leitner a écrit :
> On Wed, May 31, 2017 at 10:38:21AM +0200, Nicolas Dichtel wrote:
>> Le 30/05/2017 à 23:33, Flavio Leitner a écrit :
>>> Don't include netns id for notifications broadcasts when the
>>> socket and the skb are in the same netns because it will be
>>> an error which can't be distinguished from a peer netns failing
>>> to allocate an id.
>> I don't understand the problem. peernet2id() doesn't allocate ids, it only do a
>> lookup. If you need an id for the current netns, you have to allocate one.
>
> The issue is that if you query an interface on the same netns, the
> error is returned, then we cannot tell if the iface is on the same
> netns or if there was an error while allocating the ID and the
> iface is on another netns.
If the returned id is NETNSA_NSID_NOT_ASSIGNED, then the netns is the same.
Some lines before your patch, we call peernet_has_id() when the netns differ,
thus we ensure that the id is available.
The principle was that netlink messages of other netns can be sent only if an id
is assigned.
>
>> This patch changes the metadata exported to the userland and will break existing
>> tools.
>
> It should not break because it changes only for interfaces on
> the same netns where there is no ID and that value wasn't
> exported until recently.
>
It was exported since the initial patch (59324cf35aba ("netlink: allow to listen
"all" netns"). Am I wrong?
Regards,
Nicolas
^ permalink raw reply
* Re: [i40e] regression on TCP stream and TCP maerts, kernel-4.12.0-0.rc2
From: Adrian Tomasov @ 2017-05-31 13:48 UTC (permalink / raw)
To: Alexander Duyck, Adam Okuliar, Mitch Williams, intel-wired-lan
Cc: Netdev, Jeff Kirsher, Otto Sabart, Jirka Hladky
In-Reply-To: <CAKgT0Ufb41o=WM=3DM9cwvrw9N5AFn-uL0p1gQB-m+D3BX3DKA@mail.gmail.com>
On Tue, 2017-05-30 at 18:27 -0700, Alexander Duyck wrote:
> On Tue, May 30, 2017 at 8:41 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> >
> > On Tue, May 30, 2017 at 6:43 AM, Adam Okuliar <aokuliar@redhat.com>
> > wrote:
> > >
> > > Hello,
> > >
> > > we found regression on intel card(XL710) with i40e driver.
> > > Regression is
> > > about ~45%
> > > on TCP_STREAM and TCP_MAERTS test for IPv4 and IPv6. Regression
> > > was first
> > > visible in kernel-4.12.0-0.rc1.
> > >
> > > More details about results you can see in uploaded images in
> > > bugzilla. [0]
> > >
> > >
> > > [0] https://bugzilla.kernel.org/show_bug.cgi?id=195923
> > >
> > >
> > > Best regards, / S pozdravom,
> > >
> > > Adrián Tomašov
> > > Kernel Performance QE
> > > atomasov@redhat.com
> >
> > I have added the i40e driver maintainer and the intel-wired-lan
> > mailing list so that we can make are developers aware of the issue.
> >
> > Thanks.
> >
> > - Alex
>
> Adam,
>
> We are having some issues trying to reproduce what you reported.
>
> Can you provide some additional data. Specifically we would be
> looking
> for an "ethtool -i", and an "ethtool -S" for the port before and
> after
> the test. If you can attach it to the bugzilla that would be
> appreciated.
>
> Thanks.
>
> - Alex
Hello Alex,
requested files are updated in bugzilla.
If you have any questions about testing feel free to ask.
Best regards,
Adrian
^ permalink raw reply
* Re: [PATCH net] ipv6: xfrm: Handle errors reported by xfrm6_find_1stfragopt()
From: Craig Gallek @ 2017-05-31 14:01 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David S. Miller, netdev
In-Reply-To: <20170531121541.GA16923@decadent.org.uk>
On Wed, May 31, 2017 at 8:15 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> xfrm6_find_1stfragopt() may now return an error code and we must
> not treat it as a length.
>
> Fixes: 2423496af35d ("ipv6: Prevent overrun when parsing v6 header options")
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Commits 2423496af35d "ipv6: Prevent overrun when parsing v6 header
> options" and 7dd7eb9513bd "ipv6: Check ip6_find_1stfragopt() return
> value properly." changed ip6_find_1stfragopt() to return negative
> error codes and changed some of its callers to handle this.
>
> However, there is also xfrm6_find_1stfragopt() which is a wrapper for
> ip6_find_1stfragopt() and is called indirectly by xfrm6_ro_output()
> and xfrm6_transport_output(). Neither of them check for errors.
>
> This is totally untested. I think xfrm_type::hdr_offset deserves a
> comment about its semantics, but I don't understand it well enogugh to
> write that.
Thank you for finding this, it's a good lesson to not rely solely on cscope :\
I believe this fix is correct and sufficient. I only found 2 up-stack
callers of this (pktgen_output_ipsec and xfrm_output_one) and they
both ultimately call kfree_skb on error.
However, the fact that this function is used as a function pointer
through xfrm_type.hdr_offset made me look at a couple other functions
that can be stored in this structure. mip6_destopt_offset and
mip6_rthdr_offset have very similar implementations to the original
ip6_find_1stfragopt and may very well suffer from the same bug I was
trying to fix. Maybe it doesn't matter since that bug relied on the
user changing the v6 nexthdr field. I need to understand the mip6
code first...
In any event, I think this patch applies on its own. Thanks again.
Acked-by: Craig Gallek <kraig@google.com>
^ permalink raw reply
* Re: [PATCH net] samples/bpf: bpf_load.c order of prog_fd[] should correspond with ELF order
From: Jesper Dangaard Brouer @ 2017-05-31 14:05 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: netdev, brouer
In-Reply-To: <20170530163410.mhag3k5nssohcofd@ast-mbp.dhcp.thefacebook.com>
On Tue, 30 May 2017 09:34:12 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Tue, May 30, 2017 at 02:37:51PM +0200, Jesper Dangaard Brouer wrote:
> > An eBPF ELF file generated with LLVM can contain several program
> > section, which can be used for bpf tail calls. The bpf prog file
> > descriptors are accessible via array prog_fd[].
> >
> > At-least XDP samples assume ordering, and uses prog_fd[0] is the main
> > XDP program to attach. The actual order of array prog_fd[] depend on
> > whether or not a bpf program section is referencing any maps or not.
> > Not using a map result in being loaded/processed after all other
> > prog section. Thus, this can lead to some very strange and hard to
> > debug situation, as the user can only see a FD and cannot correlated
> > that with the ELF section name.
> >
> > The fix is rather simple, and even removes duplicate memcmp code.
> > Simply load program sections as the last step, instead of
> > load_and_attach while processing the relocation section.
> >
> > When working with tail calls, it become even more essential that the
> > order of prog_fd[] is consistant, like the current dependency of the
> > map_fd[] order.
> >
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> Looks fine, but imo net-next is better, since it's not a bugfix
> to anything in the net tree, but a general improvement.
Okay, I'm fine with this going into net-next. I'm running with own
fix[1] in my git-tree, so I can get my tail call example[2] working.
[1] https://github.com/netoptimizer/prototype-kernel/commit/e785522d84d5bf
[2] https://github.com/netoptimizer/prototype-kernel/tree/master/kernel/samples/bpf
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Thanks!
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc
From: Jia-Ju Bai @ 2017-05-31 14:23 UTC (permalink / raw)
To: Yuval.Mintz, Ariel.Elior, everest-linux-l2
Cc: netdev, linux-kernel, Jia-Ju Bai
The driver may sleep in interrupt handling, and the function call path is:
qed_int_sp_dpc (tasklet_init indicates it handles interrupt)
qed_int_attentions
qed_mcp_handle_events
qed_mcp_handle_link_change
qed_link_update
qed_fill_link
qed_mcp_get_media_type
qed_ptt_acquire
usleep_range --> may sleep
To fix it, the "usleep_range" is replaced with "udelay".
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
drivers/net/ethernet/qlogic/qed/qed_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_hw.c b/drivers/net/ethernet/qlogic/qed/qed_hw.c
index a05feb3..3250cc4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hw.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_hw.c
@@ -131,7 +131,7 @@ struct qed_ptt *qed_ptt_acquire(struct qed_hwfn *p_hwfn)
}
spin_unlock_bh(&p_hwfn->p_ptt_pool->lock);
- usleep_range(1000, 2000);
+ udelay(1500);
}
DP_NOTICE(p_hwfn, "PTT acquire timeout - failed to allocate PTT\n");
--
1.7.9.5
^ permalink raw reply related
* [PATCH] net: davicom: dm9000: Avoid spinlock recursion during dm9000_timeout routine
From: Liu Xiang @ 2017-05-31 15:06 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Liu Xiang
On the DM9000B, dm9000_phy_write() is called after the main spinlock
is held, during the dm9000_timeout() routine. Spinlock recursion
occurs because the main spinlock is requested again in
dm9000_phy_write(). So spinlock should be avoided in dm9000_phy_write()
during the dm9000_timeout() routine.
Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn>
---
drivers/net/ethernet/davicom/dm9000.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c
index 008dc81..afe33de 100644
--- a/drivers/net/ethernet/davicom/dm9000.c
+++ b/drivers/net/ethernet/davicom/dm9000.c
@@ -337,11 +337,11 @@ static void dm9000_msleep(struct board_info *db, unsigned int ms)
unsigned long reg_save;
dm9000_dbg(db, 5, "phy_write[%02x] = %04x\n", reg, value);
- if (!db->in_timeout)
+ if (!db->in_timeout) {
mutex_lock(&db->addr_lock);
- spin_lock_irqsave(&db->lock, flags);
-
+ spin_lock_irqsave(&db->lock, flags);
+ }
/* Save previous register address */
reg_save = readb(db->io_addr);
@@ -356,11 +356,13 @@ static void dm9000_msleep(struct board_info *db, unsigned int ms)
iow(db, DM9000_EPCR, EPCR_EPOS | EPCR_ERPRW);
writeb(reg_save, db->io_addr);
- spin_unlock_irqrestore(&db->lock, flags);
+ if (!db->in_timeout)
+ spin_unlock_irqrestore(&db->lock, flags);
dm9000_msleep(db, 1); /* Wait write complete */
- spin_lock_irqsave(&db->lock, flags);
+ if (!db->in_timeout)
+ spin_lock_irqsave(&db->lock, flags);
reg_save = readb(db->io_addr);
iow(db, DM9000_EPCR, 0x0); /* Clear phyxcer write command */
@@ -368,9 +370,11 @@ static void dm9000_msleep(struct board_info *db, unsigned int ms)
/* restore the previous address */
writeb(reg_save, db->io_addr);
- spin_unlock_irqrestore(&db->lock, flags);
- if (!db->in_timeout)
+ if (!db->in_timeout) {
+ spin_unlock_irqrestore(&db->lock, flags);
+
mutex_unlock(&db->addr_lock);
+ }
}
/* dm9000_set_io
--
1.9.1
^ permalink raw reply related
* Re: [PULL] topic/e1000e-fix
From: David Miller @ 2017-05-31 15:08 UTC (permalink / raw)
To: daniel.vetter
Cc: airlied, intel-gfx, dri-devel, jani.nikula, jeffrey.t.kirsher,
intel-wired-lan, netdev, linux-kernel
In-Reply-To: <CAKMK7uGbi2Bwfno9XogKZ-LUOyMepbtRyGuGrNYWwj6Cts+uag@mail.gmail.com>
From: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed, 31 May 2017 08:10:45 +0200
> On Wed, May 31, 2017 at 7:54 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> On Wed, May 31, 2017 at 1:06 AM, Dave Airlie <airlied@gmail.com> wrote:
>>> On 31 May 2017 at 08:10, David Miller <davem@davemloft.net> wrote:
>>>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Date: Tue, 30 May 2017 22:15:42 +0200
>>>>
>>>>> If the e1000e maintainer wants to coalesce or not return statements
>>>>> this simple way, that's imo on him to change the color as needed.
>>>>
>>>> That's not how things work.
>>>>
>>>> If the maintainer wants you to style things a certain way, either you
>>>> do it that way or your patch isn't accepted.
>>
>> Consider this pull a regression report, pls handle it.
>
> And I guess I pile of more cc, to make this regression report
> complete. I mean you got the backtrace, bisect and a proposed fix, and
> the almost-whitespace change demanded is something gcc does in its
> sleep. I'd understand a request to retest if it would be a real
> functional change, but in this situation I have no idea why this
> regression just can't be fixed already.
And we can't understand why respinning with the requested change is
less work than making several postings such as this one.
^ permalink raw reply
* Re: [PATCH] net: rtnetlink: bail out from rtnl_fdb_dump() on parse error
From: David Miller @ 2017-05-31 15:10 UTC (permalink / raw)
To: glider; +Cc: dvyukov, kcc, edumazet, linux-kernel, netdev
In-Reply-To: <CAG_fn=WNnkmZ8CXqM6w7p_kmhh8Tys1HjgkfASYjvEZ2iie0hg@mail.gmail.com>
From: Alexander Potapenko <glider@google.com>
Date: Wed, 31 May 2017 10:56:47 +0200
> Hi David,
>
> I've noticed that the upstream patch:
> https://github.com/torvalds/linux/commit/0ff50e83b5122e836ca492fefb11656b225ac29c
> contains the KMSAN report and the repro, despite I've put them under
> the triple dash (IIRC Eric told me I shouldn't bloat the patch
> descriptions with that information). Did I mess it up somehow?
I put them into the main commit log message by hand when I applied the
patch, I think the more information in commit log messages the better.
^ permalink raw reply
* [PATCH net-next 0/9] nfp: move BPF offload code into app
From: Jakub Kicinski @ 2017-05-31 15:06 UTC (permalink / raw)
To: netdev; +Cc: oss-drivers, Jakub Kicinski
Hi!
This series moves the eBPF offload code out of netdev/vNIC handling and
starts building the nfp_app. Port init is moved into the apps as well
because various apps associate vNICs, representors with ports differently.
First patch adds a helper for updating tc stats which has been waiting
in my tree to be included in any moderately related series.
Next series will bring communicating with FW using control messages,
then representors, BPF maps, tc flower... :)
Jakub Kicinski (9):
sched: add helper for updating statistics on all actions
nfp: add missing fall through statements
nfp: turn reading PCIe RTsym parameters into a helper
nfp: move port init to apps
nfp: report app name in ethtool -i
nfp: move eBPF offload files to BPF app directory
nfp: move bpf offload code to the BPF app
nfp: move basic eBPF stats to app-specific code
nfp: fix memory leak on FW load error
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 10 +-
.../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 10 +-
drivers/net/ethernet/netronome/nfp/Makefile | 11 +-
.../netronome/nfp/{nfp_bpf_jit.c => bpf/jit.c} | 4 +-
drivers/net/ethernet/netronome/nfp/bpf/main.c | 160 +++++++++++++++++++++
.../netronome/nfp/{nfp_bpf.h => bpf/main.h} | 23 +++
.../nfp/{nfp_net_offload.c => bpf/offload.c} | 61 ++++----
.../nfp/{nfp_bpf_verifier.c => bpf/verifier.c} | 2 +-
drivers/net/ethernet/netronome/nfp/nfp_app.c | 21 ++-
drivers/net/ethernet/netronome/nfp/nfp_app.h | 120 +++++++++++++++-
drivers/net/ethernet/netronome/nfp/nfp_app_nic.c | 86 +++++++++++
drivers/net/ethernet/netronome/nfp/nfp_asm.h | 2 +-
drivers/net/ethernet/netronome/nfp/nfp_main.c | 1 +
drivers/net/ethernet/netronome/nfp/nfp_main.h | 6 +
drivers/net/ethernet/netronome/nfp/nfp_net.h | 16 +--
.../net/ethernet/netronome/nfp/nfp_net_common.c | 71 ++-------
.../net/ethernet/netronome/nfp/nfp_net_ethtool.c | 5 +-
drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 86 ++++++-----
drivers/net/ethernet/netronome/nfp/nic/main.c | 58 ++++++++
include/net/pkt_cls.h | 19 +++
20 files changed, 600 insertions(+), 172 deletions(-)
rename drivers/net/ethernet/netronome/nfp/{nfp_bpf_jit.c => bpf/jit.c} (99%)
create mode 100644 drivers/net/ethernet/netronome/nfp/bpf/main.c
rename drivers/net/ethernet/netronome/nfp/{nfp_bpf.h => bpf/main.h} (88%)
rename drivers/net/ethernet/netronome/nfp/{nfp_net_offload.c => bpf/offload.c} (85%)
rename drivers/net/ethernet/netronome/nfp/{nfp_bpf_verifier.c => bpf/verifier.c} (99%)
create mode 100644 drivers/net/ethernet/netronome/nfp/nfp_app_nic.c
create mode 100644 drivers/net/ethernet/netronome/nfp/nic/main.c
--
2.11.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox