* [PATCH net-next v2 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-27 16:58 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, James Morris, Trond Myklebust, Alexander Duyck,
Daniel Borkmann, Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
Edward Cree, linux-nfs
In-Reply-To: <cover.1474995024.git.pabeni@redhat.com>
Avoid usage of common memory accounting functions, since
the logic is pretty much different.
To account for forward allocation, a couple of new atomic_t
members are added to udp_sock: 'mem_alloced' and 'can_reclaim'.
The current forward allocation is estimated as 'mem_alloced'
minus 'sk_rmem_alloc'.
When the forward allocation can't cope with the packet to be
enqueued, 'mem_alloced' is incremented by the packet size
rounded-up to the next SK_MEM_QUANTUM.
After a dequeue, if under memory pressure, we try to partially
reclaim all forward allocated memory rounded down to an
SK_MEM_QUANTUM and 'mem_alloc' is decreased by that amount.
To protect against concurrent reclaim, we use 'can_reclaim' as
an unblocking synchronization point and let only one process
do the work.
sk->sk_forward_alloc is set after each memory update to the
currently estimated forward allocation, without any lock or
protection.
This value is updated/maintained only to expose some
semi-reasonable value to the eventual reader, and is guaranteed
to be 0 at socket destruction time.
The above needs custom memory reclaiming on shutdown, provided
by the udp_destruct_sock() helper, which completely reclaim
the allocated forward memory.
v1 -> v2:
- use a udp specific destrctor to perform memory reclaiming
- remove a couple of helpers, unneeded after the above cleanup
- do not reclaim memory on dequeue if not under memory
pressure
- reworked the fwd accounting schema to avoid potential
integer overflow
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/udp.h | 2 +
include/net/udp.h | 7 +++
net/ipv4/udp.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 146 insertions(+)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index d1fd8cd..16aa22b 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -42,6 +42,8 @@ static inline u32 udp_hashfn(const struct net *net, u32 num, u32 mask)
struct udp_sock {
/* inet_sock has to be the first member */
struct inet_sock inet;
+ atomic_t mem_allocated;
+ atomic_t can_reclaim;
#define udp_port_hash inet.sk.__sk_common.skc_u16hashes[0]
#define udp_portaddr_hash inet.sk.__sk_common.skc_u16hashes[1]
#define udp_portaddr_node inet.sk.__sk_common.skc_portaddr_node
diff --git a/include/net/udp.h b/include/net/udp.h
index ea53a87..b9563ef 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -98,6 +98,8 @@ static inline struct udp_hslot *udp_hashslot2(struct udp_table *table,
extern struct proto udp_prot;
extern atomic_long_t udp_memory_allocated;
+extern int udp_memory_pressure;
+extern struct percpu_counter udp_sockets_allocated;
/* sysctl variables for udp */
extern long sysctl_udp_mem[3];
@@ -246,6 +248,10 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb,
}
/* net/ipv4/udp.c */
+void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len);
+int udp_rmem_schedule(struct sock *sk, struct sk_buff *skb);
+void udp_enter_memory_pressure(struct sock *sk);
+
void udp_v4_early_demux(struct sk_buff *skb);
int udp_get_port(struct sock *sk, unsigned short snum,
int (*saddr_cmp)(const struct sock *,
@@ -258,6 +264,7 @@ void udp_flush_pending_frames(struct sock *sk);
void udp4_hwcsum(struct sk_buff *skb, __be32 src, __be32 dst);
int udp_rcv(struct sk_buff *skb);
int udp_ioctl(struct sock *sk, int cmd, unsigned long arg);
+int udp_init_sock(struct sock *sk);
int udp_disconnect(struct sock *sk, int flags);
unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait);
struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 7d96dc2..2218901 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -131,6 +131,12 @@ EXPORT_SYMBOL(sysctl_udp_wmem_min);
atomic_long_t udp_memory_allocated;
EXPORT_SYMBOL(udp_memory_allocated);
+int udp_memory_pressure __read_mostly;
+EXPORT_SYMBOL(udp_memory_pressure);
+
+struct percpu_counter udp_sockets_allocated;
+EXPORT_SYMBOL(udp_sockets_allocated);
+
#define MAX_UDP_PORTS 65536
#define PORTS_PER_CHAIN (MAX_UDP_PORTS / UDP_HTABLE_SIZE_MIN)
@@ -1172,6 +1178,137 @@ out:
return ret;
}
+static int __udp_forward(struct udp_sock *up, int rmem)
+{
+ return atomic_read(&up->mem_allocated) - rmem;
+}
+
+static bool udp_under_memory_pressure(const struct sock *sk)
+{
+ if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
+ mem_cgroup_under_socket_pressure(sk->sk_memcg))
+ return true;
+
+ return READ_ONCE(udp_memory_pressure);
+}
+
+void udp_enter_memory_pressure(struct sock *sk)
+{
+ WRITE_ONCE(udp_memory_pressure, 1);
+}
+EXPORT_SYMBOL(udp_enter_memory_pressure);
+
+/* if partial != 0 do nothing if not under memory pressure and avoid
+ * reclaiming last quanta
+ */
+static void udp_rmem_release(struct sock *sk, int partial)
+{
+ struct udp_sock *up = udp_sk(sk);
+ int fwd, amt;
+
+ if (partial && !udp_under_memory_pressure(sk))
+ return;
+
+ /* we can have concurrent release; if we catch any conflict
+ * we let only one of them do the work
+ */
+ if (atomic_dec_if_positive(&up->can_reclaim) < 0)
+ return;
+
+ fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
+ if (fwd < SK_MEM_QUANTUM + partial) {
+ atomic_inc(&up->can_reclaim);
+ return;
+ }
+
+ amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
+ atomic_sub(amt, &up->mem_allocated);
+ atomic_inc(&up->can_reclaim);
+
+ __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
+ sk->sk_forward_alloc = fwd - amt;
+}
+
+static void udp_rmem_free(struct sk_buff *skb)
+{
+ struct sock *sk = skb->sk;
+
+ atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+ udp_rmem_release(sk, 1);
+}
+
+int udp_rmem_schedule(struct sock *sk, struct sk_buff *skb)
+{
+ int fwd, amt, delta, rmem, err = -ENOMEM;
+ struct udp_sock *up = udp_sk(sk);
+
+ rmem = atomic_add_return(skb->truesize, &sk->sk_rmem_alloc);
+ if (rmem > sk->sk_rcvbuf)
+ goto drop;
+
+ fwd = __udp_forward(up, rmem);
+ if (fwd > 0)
+ goto no_alloc;
+
+ amt = sk_mem_pages(skb->truesize);
+ delta = amt << SK_MEM_QUANTUM_SHIFT;
+ if (!__sk_mem_raise_allocated(sk, delta, amt, SK_MEM_RECV)) {
+ err = -ENOBUFS;
+ goto drop;
+ }
+
+ /* if we have some skbs in the error queue, the forward allocation could
+ * be understimated, even below 0; avoid exporting such values
+ */
+ fwd = atomic_add_return(delta, &up->mem_allocated) - rmem;
+ if (fwd < 0)
+ fwd = SK_MEM_QUANTUM;
+
+no_alloc:
+ sk->sk_forward_alloc = fwd;
+ skb_orphan(skb);
+ skb->sk = sk;
+ skb->destructor = udp_rmem_free;
+ return 0;
+
+drop:
+ atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+ atomic_inc(&sk->sk_drops);
+ return err;
+}
+EXPORT_SYMBOL_GPL(udp_rmem_schedule);
+
+static void udp_destruct_sock(struct sock *sk)
+{
+ /* reclaim completely the forward allocated memory */
+ __skb_queue_purge(&sk->sk_receive_queue);
+ udp_rmem_release(sk, 0);
+ inet_sock_destruct(sk);
+}
+
+int udp_init_sock(struct sock *sk)
+{
+ struct udp_sock *up = udp_sk(sk);
+
+ atomic_set(&up->mem_allocated, 0);
+ atomic_set(&up->can_reclaim, 1);
+ sk->sk_destruct = udp_destruct_sock;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(udp_init_sock);
+
+void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len)
+{
+ if (unlikely(READ_ONCE(sk->sk_peek_off) >= 0)) {
+ bool slow = lock_sock_fast(sk);
+
+ sk_peek_offset_bwd(sk, len);
+ unlock_sock_fast(sk, slow);
+ }
+ consume_skb(skb);
+}
+EXPORT_SYMBOL_GPL(skb_consume_udp);
+
/**
* first_packet_length - return length of first packet in receive queue
* @sk: socket
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH net-next 0/2] net: ethernet: mediatek: some bug fixes for PDAM and HW LRO
From: Nelson Chang @ 2016-09-27 17:16 UTC (permalink / raw)
To: davem; +Cc: john, nbd, netdev, linux-mediatek, nelsonch.tw
Got it. I'll notice that for later patches.
Thanks David.
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, September 27, 2016 9:42 PM
To: Nelson Chang (張家祥)
Cc: john@phrozen.org; nbd@openwrt.org; netdev@vger.kernel.org;
linux-mediatek@lists.infradead.org; nelsonch.tw@gmail.com
Subject: Re: [PATCH net-next 0/2] net: ethernet: mediatek: some bug
fixes for PDAM and HW LRO
From: Nelson Chang <nelson.chang@mediatek.com>
Date: Mon, 26 Sep 2016 14:33:48 +0800
> 1) Add to stop PDMA while stopping the frame engine
> 2) Modify the register settings for LRO relinquishments
> 3) Jump out from the waiting loop while LRO relinquishments are done
Series applied, but like Sergei I think you should have split patch
#2 into two separate patches.
You even list the changes individually here in your header posting.
^ permalink raw reply
* [PATCH net] net: skbuff: Fix incorrect skb->mac_len adjustment in skb_vlan_push()
From: Shmulik Ladkani @ 2016-09-27 17:31 UTC (permalink / raw)
To: David S . Miller, Pravin Shelar
Cc: Jiri Pirko, Daniel Borkmann, netdev, Shmulik Ladkani
In case 'skb_vlan_push' is called on an skb with a hw-accel vlan tag
already present, the existing hw-accel tag is inserted into payload, and
the new given tag is placed as new hw-accel tag.
After the insertion:
- 'mac_header' is adjusted to point to the new start of the vlan_ethhdr
- 'data' is adjusted to point to the vlan_hdr portion
(since packet's payload is the inner 802.1q)
However, 'mac_len' is incorrectly incremented with additional VLAN_HLEN
bytes, resulting in a total value of 18 bytes.
Meaning, when issuing 'skb_push(skb, skb->mac_len)' the data points
to random content, 4 bytes PRIOR the ethhdr location.
This is problematic, as many constructs in the stack are issuing
'skb_push(skb, skb->mac_len)' prior xmit to a device (e.g tcf_mirred,
tcf_bpf, nf_dup_netdev_egress), resulting in bogus frames being
xmitted (having random 4 bytes at start of frame).
For example:
# ip l add dev d0 type dummy
# tc filter add dev eth0 parent ffff: pref 1 basic \
action vlan push protocol 802.1ad id 5 pipe \
action mirred egress redirect dev d0
Any 802.1q (hw-accel) tagged frames arriving on eth0 are xmitted as
bogus frames on d0; whereas the expected behavior is having QinQ frames.
Fix, removing the unnecessary VLAN_HLEN adjustment of mac_len.
Fixes: 93515d53b1 ("net: move vlan pop/push functions into common code")
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Pravin Shelar <pshelar@ovn.org>
Cc: Jiri Pirko <jiri@mellanox.com>
---
- David, if patch ok, suggest this goes to -stable
- Pravin, original push_vlan() code in openvswitch/actions.c prior Jiri
has moved it into skbuff.c had the following comment:
/* Update mac_len for subsequent MPLS actions */
skb->mac_len += VLAN_HLEN;
Can you please acknowlegde OvS code is also ok with suggested change?
net/core/skbuff.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d36c754895..0cf961868b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4607,7 +4607,6 @@ int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
}
skb->protocol = skb->vlan_proto;
- skb->mac_len += VLAN_HLEN;
skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
__skb_pull(skb, offset);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH resend] sh_eth: add R8A7743/5 support
From: Sergei Shtylyov @ 2016-09-27 18:08 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: netdev@vger.kernel.org, Linux-Renesas, Rob Herring, Mark Rutland,
devicetree@vger.kernel.org
In-Reply-To: <CAMuHMdWq_714eP2q7V6AgopKu-mNx0GLUzyemad2H_ZcFt15ug@mail.gmail.com>
On 09/27/2016 10:35 AM, Geert Uytterhoeven wrote:
>> Add support for the first two members of the Renesas RZ/G family, RZ/G1M/E
>> (also known as R8A7743/5). The Ether core is the same as in the R-Car gen2
>> SoCs, so will share the code/data with them...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>> --- net-next.orig/drivers/net/ethernet/renesas/Kconfig
>> +++ net-next/drivers/net/ethernet/renesas/Kconfig
>> @@ -27,7 +27,7 @@ config SH_ETH
>> Renesas SuperH Ethernet device driver.
>> This driver supporting CPUs are:
>> - SH7619, SH7710, SH7712, SH7724, SH7734, SH7763, SH7757,
>> - R8A7740, R8A777x and R8A779x.
>> + R8A7740, R8A774x, R8A777x and R8A779x.
>
> Surely "R8A7740" is covered by "R8A774x"? :-)
It should be, yes -- but 7740 has completely different Ether core than the
RZ/G family. I can fix this if you want...
> However, the "x" is not a real wildcard (also for '7x and '9x), as the driver
> doesn't support all possible values of "x".
Well, I think for 779x it does, provided that the Ether core exists at all.
For 777x it doesn't support the SH flavour of 7778 indeed but it's not
supported by the kernel at all...
> Apart from that:
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Thank you. :-)
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
^ permalink raw reply
* Re: [RFC PATCH net-next 2/2] sfc: report 4-tuple UDP hashing to ethtool, if it's enabled
From: Mintz, Yuval @ 2016-09-27 18:12 UTC (permalink / raw)
To: Edward Cree, linux-net-drivers@solarflare.com,
netdev@vger.kernel.org, davem@davemloft.net
Cc: bkenward@solarflare.com
In-Reply-To: <8341c601-674a-74ff-c6dd-689c19b3ce7f@solarflare.com>
> info->data = 0;
> switch (info->flow_type) {
> + case UDP_V4_FLOW:
> + if (efx->rx_hash_udp_4tuple)
> + /* fall through */
> + /* else fall further! */
> case TCP_V4_FLOW:
> - info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
> /* fall through */
> - case UDP_V4_FLOW:
> case SCTP_V4_FLOW:
> case AH_ESP_V4_FLOW:
> case IPV4_FLOW:
> info->data |= RXH_IP_SRC | RXH_IP_DST;
> min_revision = EFX_REV_FALCON_B0;
> break;
Well, you sure fulfilled your cover letter's promise. ;-)
Do you really prefer this conditional mayham over copy-pasting some lines?
^ permalink raw reply
* Re: [net-next 5/5] PCI: disable FLR for 82579 device
From: Bjorn Helgaas @ 2016-09-27 18:17 UTC (permalink / raw)
To: Neftin, Sasha
Cc: Jeff Kirsher, linux-pci, davem, bhelgaas, netdev, nhorman,
sassmann, jogreene, guru.anbalagane
In-Reply-To: <176f2366-e225-75fb-8cad-909a8f7e808c@intel.com>
On Sun, Sep 25, 2016 at 10:02:43AM +0300, Neftin, Sasha wrote:
> On 9/24/2016 12:05 AM, Jeff Kirsher wrote:
> >On Fri, 2016-09-23 at 09:01 -0500, Bjorn Helgaas wrote:
> >>On Thu, Sep 22, 2016 at 11:39:01PM -0700, Jeff Kirsher wrote:
> >>>From: Sasha Neftin <sasha.neftin@intel.com>
> >>>
> >>>82579 has a problem reattaching itself after the device is detached.
> >>>The bug was reported by Redhat. The suggested fix is to disable
> >>>FLR capability in PCIe configuration space.
> >>>
> >>>Reproduction:
> >>>Attach the device to a VM, then detach and try to attach again.
> >>>
> >>>Fix:
> >>>Disable FLR capability to prevent the 82579 from hanging.
> >>Is there a bugzilla or other reference URL to include here? Should
> >>this be marked for stable?
> >So the author is in Israel, meaning it is their weekend now. I do not
> >believe Sasha monitors email over the weekend, so a response to your
> >questions won't happen for a few days.
> >
> >I tried searching my archives for more information, but had no luck finding
> >any additional information.
> >
> >>>Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> >>>Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> >>>Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >>>---
> >>> drivers/pci/quirks.c | 21 +++++++++++++++++++++
> >>> 1 file changed, 21 insertions(+)
> >>>
> >>>diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >>>index 44e0ff3..59fba6e 100644
> >>>--- a/drivers/pci/quirks.c
> >>>+++ b/drivers/pci/quirks.c
> >>>@@ -4431,3 +4431,24 @@ static void quirk_intel_qat_vf_cap(struct
> >>>pci_dev *pdev)
> >>> }
> >>> }
> >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> >>>quirk_intel_qat_vf_cap);
> >>>+/*
> >>>+ * Workaround FLR issues for 82579
> >>>+ * This code disables the FLR (Function Level Reset) via PCIe, in
> >>>order
> >>>+ * to workaround a bug found while using device passthrough, where the
> >>>+ * interface would become non-responsive.
> >>>+ * NOTE: the FLR bit is Read/Write Once (RWO) in config space, so if
> >>>+ * the BIOS or kernel writes this register * then this workaround will
> >>>+ * not work.
> >>This doesn't sound like a root cause. Is the issue a hardware
> >>erratum? Linux PCI core bug? VFIO bug? Device firmware bug?
> >>
> >>The changelog suggests that the problem only affects passthrough,
> >>which suggests some sort of kernel bug related to how passthrough is
> >>implemented.
If this bug affects all scenarios, not just passthrough, the changelog
should not mention passthrough.
> >>>+ */
> >>>+static void quirk_intel_flr_cap_dis(struct pci_dev *dev)
> >>>+{
> >>>+ int pos = pci_find_capability(dev, PCI_CAP_ID_AF);
> >>>+ if (pos) {
> >>>+ u8 cap;
> >>>+ pci_read_config_byte(dev, pos + PCI_AF_CAP, &cap);
> >>>+ cap = cap & (~PCI_AF_CAP_FLR);
> >>>+ pci_write_config_byte(dev, pos + PCI_AF_CAP, cap);
> >>>+ }
> >>>+}
> >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502,
> >>>quirk_intel_flr_cap_dis);
> >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503,
> >>>quirk_intel_flr_cap_dis);
> >>>--
> >>>2.7.4
> >>>
> >>>--
> >>>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> >>>the body of a message to majordomo@vger.kernel.org
> >>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Hello,
>
> Original bugzilla thread could be found here:
> https://bugzilla.redhat.com/show_bug.cgi?format=multiple&id=966840
That bugzilla is private and I can't read it.
> This is our HW bug, exist only in 82579 devices. More new devices
> have no such problem. We have found root cause and suggested this
> solution.
Is there an erratum you can reference?
> This solution should work for a 95% of cases, so I do not
> think that this is fragile. For another cases possible solution is
> get up working system and manually disable FLR, before VM start use
> our adapter.
I don't think a 95% solution is sufficient. Can you use the
pci_dev_specific_reset() framework to make a 100% solution?
Bjorn
^ permalink raw reply
* Re: [PATCH net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit
From: Shmulik Ladkani @ 2016-09-27 18:24 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
netdev, Shmulik Ladkani
In-Reply-To: <57EA4A3C.8000508@iogearbox.net>
Hi,
On Tue, 27 Sep 2016 12:30:20 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 09/22/2016 03:21 PM, Shmulik Ladkani wrote:
> > From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> >
> > 'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
> > egress to the target device (if action is performed at ingress).
> >
> > Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
> > the target device.
> > This allows to decouple the attribute from the action to be taken.
> >
> > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> > ---
> > include/net/tc_act/tc_mirred.h | 2 +-
> > net/sched/act_mirred.c | 10 +++++-----
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
> > index 62770ad..5275158 100644
> > --- a/include/net/tc_act/tc_mirred.h
> > +++ b/include/net/tc_act/tc_mirred.h
> > @@ -8,7 +8,7 @@ struct tcf_mirred {
> > struct tc_action common;
> > int tcfm_eaction;
> > int tcfm_ifindex;
> > - int tcfm_ok_push;
> > + int tcfm_mac_header_xmit;
>
> Since you already touch this here and in patch 2/4 anyway, maybe
> make that a bool along the way?
Ok.
(Thought of it, but my urge to lessen the diff eventually won)
> Perhaps instead of tcfm_mac_header_xmit, tcfm_mac_header_push
> might be a better name?
Don't think so.
Eventually this serves as the decision to either push or pull, so prefer
not to name it as the action (push/pull) but rather what is target
device's property (xmits at mh?).
^ permalink raw reply
* [PATCH RFC net-next] bnx2x: avoid printing unnecessary messages during register dump
From: Guilherme G. Piccoli @ 2016-09-27 18:33 UTC (permalink / raw)
To: ariel.elior, Yuval.Mintz; +Cc: netdev, gpiccoli
The bnx2x driver prints multiple error messages during register dump,
with "ethtool -d" for example. The driver even warn that many messages
might be seen during the register dump, but they are harmless. A typical
kernel log after register dump looks like this:
[9.375] bnx2x: [bnx2x_get_regs:987(net0)]Generating register dump. Might trigger harmless GRC timeouts
[9.439] bnx2x: [bnx2x_attn_int_deasserted3:4342(net0)]LATCHED attention 0x04000000 (masked)
[9.439] bnx2x: [bnx2x_attn_int_deasserted3:4346(net0)]GRC time-out 0x010580cd
[...]
The notation [...] means that some messages were supressed - in our
tests we saw 78 more "LATCHED attention" and "GRC time-out" messages,
supressed here.
This patch avoid these messages to be printed on register dump instead
of just warn they are harmless.
Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
* This was sent as RFC for two main reasons: firstly, I might be ignoring
some importance in showing these error messages during register dump.
Also, there are multiple ways to implement this idea - I just did the
first one that came to my head. We might also add a new flag on struct
bnx2x or even a new field. Any suggestions regarding the best
implementation are welcome.
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 1 +
.../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 17 ++++++++++++-----
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 22 ++++++++++++----------
3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 7dd7490..73f2713 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -2053,6 +2053,7 @@ void bnx2x_update_coalesce(struct bnx2x *bp);
int bnx2x_get_cur_phy_idx(struct bnx2x *bp);
bool bnx2x_port_after_undi(struct bnx2x *bp);
+bool bnx2x_is_reading_regs(void);
static inline u32 reg_poll(struct bnx2x *bp, u32 reg, u32 expected, int ms,
int wait)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 85a7800..d7dc867 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -29,6 +29,8 @@
#include "bnx2x_dump.h"
#include "bnx2x_init.h"
+static int bnx2x_reading_regs;
+
/* Note: in the format strings below %s is replaced by the queue-name which is
* either its index or 'fcoe' for the fcoe queue. Make sure the format string
* length does not exceed ETH_GSTRING_LEN - MAX_QUEUE_NAME_LEN + 2
@@ -981,19 +983,24 @@ static void bnx2x_get_regs(struct net_device *dev,
memcpy(p, &dump_hdr, sizeof(struct dump_header));
p += dump_hdr.header_size + 1;
- /* This isn't really an error, but since attention handling is going
- * to print the GRC timeouts using this macro, we use the same.
+ /* Actually read the registers - we use bnx2x_reading_regs to
+ * avoid multiple unnecessary error messages to be printed on
+ * kernel log when reading registers, like GRC timeouts.
*/
- BNX2X_ERR("Generating register dump. Might trigger harmless GRC timeouts\n");
-
- /* Actually read the registers */
+ bnx2x_reading_regs = 1;
__bnx2x_get_regs(bp, p);
+ bnx2x_reading_regs = 0;
/* Re-enable parity attentions */
bnx2x_clear_blocks_parity(bp);
bnx2x_enable_blocks_parity(bp);
}
+inline bool bnx2x_is_reading_regs(void)
+{
+ return !!bnx2x_reading_regs;
+}
+
static int bnx2x_get_preset_regs_len(struct net_device *dev, u32 preset)
{
struct bnx2x *bp = netdev_priv(dev);
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index fa3386b..392d14c 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -4339,16 +4339,18 @@ static void bnx2x_attn_int_deasserted3(struct bnx2x *bp, u32 attn)
}
if (attn & EVEREST_LATCHED_ATTN_IN_USE_MASK) {
- BNX2X_ERR("LATCHED attention 0x%08x (masked)\n", attn);
- if (attn & BNX2X_GRC_TIMEOUT) {
- val = CHIP_IS_E1(bp) ? 0 :
- REG_RD(bp, MISC_REG_GRC_TIMEOUT_ATTN);
- BNX2X_ERR("GRC time-out 0x%08x\n", val);
- }
- if (attn & BNX2X_GRC_RSV) {
- val = CHIP_IS_E1(bp) ? 0 :
- REG_RD(bp, MISC_REG_GRC_RSV_ATTN);
- BNX2X_ERR("GRC reserved 0x%08x\n", val);
+ if (!bnx2x_is_reading_regs()) {
+ BNX2X_ERR("LATCHED attention 0x%08x (masked)\n", attn);
+ if (attn & BNX2X_GRC_TIMEOUT) {
+ val = CHIP_IS_E1(bp) ? 0 :
+ REG_RD(bp, MISC_REG_GRC_TIMEOUT_ATTN);
+ BNX2X_ERR("GRC time-out 0x%08x\n", val);
+ }
+ if (attn & BNX2X_GRC_RSV) {
+ val = CHIP_IS_E1(bp) ? 0 :
+ REG_RD(bp, MISC_REG_GRC_RSV_ATTN);
+ BNX2X_ERR("GRC reserved 0x%08x\n", val);
+ }
}
REG_WR(bp, MISC_REG_AEU_CLR_LATCH_SIGNAL, 0x7ff);
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next v2 3/3] udp: use it's own memory accounting schema
From: Eric Dumazet @ 2016-09-27 18:42 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <bc55b0885e0f93895c211168561a6b3403bade10.1474995024.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, 2016-09-27 at 18:58 +0200, Paolo Abeni wrote:
>
> Since the new memory accounting model does not require socket
> locking, remove the lock on enqueue and free and avoid using the
> backlog on enqueue.
...
> __UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite);
> @@ -2345,6 +2325,7 @@ struct proto udp_prot = {
> .connect = ip4_datagram_connect,
> .disconnect = udp_disconnect,
> .ioctl = udp_ioctl,
> + .init = udp_init_sock,
> .destroy = udp_destroy_sock,
> .setsockopt = udp_setsockopt,
> .getsockopt = udp_getsockopt,
> @@ -2357,7 +2338,10 @@ struct proto udp_prot = {
> .unhash = udp_lib_unhash,
> .rehash = udp_v4_rehash,
> .get_port = udp_v4_get_port,
> + .enter_memory_pressure = udp_enter_memory_pressure,
> + .sockets_allocated = &udp_sockets_allocated,
> .memory_allocated = &udp_memory_allocated,
> + .memory_pressure = &udp_memory_pressure,
> .sysctl_mem = sysctl_udp_mem,
> .sysctl_wmem = &sysctl_udp_wmem_min,
> .sysctl_rmem = &sysctl_udp_rmem_min,
I find disturbing you did not remove
.backlog_rcv = __udp_queue_rcv_skb,
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: Adit Ranadive @ 2016-09-27 18:50 UTC (permalink / raw)
To: David Laight, Yuval Shaia
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org,
pv-drivers@vmware.com, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, jhansen@vmware.com,
asarwade@vmware.com, georgezhang@vmware.com, bryantan@vmware.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB010A6EA@AcuExch.aculab.com>
On Tue, Sep 27, 2016 at 09:21:27AM +0000, David Laight wrote:
> From: Adit Ranadive
> > Sent: 26 September 2016 19:15
> > On Mon, Sep 26, 2016 at 00:27:40AM -0700, Yuval Shaia wrote:
> > > On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > > > +
> > > > + /* Currently, the driver only supports RoCE mode. */
> > > > + if (dev->dsr->caps.mode != PVRDMA_DEVICE_MODE_ROCE) {
> > > > + dev_err(&pdev->dev, "unsupported transport %d\n",
> > > > + dev->dsr->caps.mode);
> > > > + ret = -EINVAL;
> > >
> > > This is some fatal error with the device, not that something wrong with the
> > > function's argument.
> > > Suggesting to replace with -EFAULT.
> > >
> >
> > Thanks, will fix this one and the others here.
>
> Won't EFAULT generate SIGSEGV ?
Since this is called at module load time, wouldn't the module load fail with
this error rather than generate a SIGSEGV?
I'm slightly unclear about what would if it is compiled into the kernel though
I think it should fail with the error.
The only other error value to return here that could make sense is EIO.
^ permalink raw reply
* Re: [net-next 5/5] PCI: disable FLR for 82579 device
From: Alex Williamson @ 2016-09-27 19:13 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Neftin, Sasha, Jeff Kirsher, linux-pci, davem, bhelgaas, netdev,
nhorman, sassmann, jogreene, guru.anbalagane
In-Reply-To: <20160927181702.GA7275@localhost>
On Tue, 27 Sep 2016 13:17:02 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Sun, Sep 25, 2016 at 10:02:43AM +0300, Neftin, Sasha wrote:
> > On 9/24/2016 12:05 AM, Jeff Kirsher wrote:
> > >On Fri, 2016-09-23 at 09:01 -0500, Bjorn Helgaas wrote:
> > >>On Thu, Sep 22, 2016 at 11:39:01PM -0700, Jeff Kirsher wrote:
> > >>>From: Sasha Neftin <sasha.neftin@intel.com>
> > >>>
> > >>>82579 has a problem reattaching itself after the device is detached.
> > >>>The bug was reported by Redhat. The suggested fix is to disable
> > >>>FLR capability in PCIe configuration space.
> > >>>
> > >>>Reproduction:
> > >>>Attach the device to a VM, then detach and try to attach again.
> > >>>
> > >>>Fix:
> > >>>Disable FLR capability to prevent the 82579 from hanging.
> > >>Is there a bugzilla or other reference URL to include here? Should
> > >>this be marked for stable?
> > >So the author is in Israel, meaning it is their weekend now. I do not
> > >believe Sasha monitors email over the weekend, so a response to your
> > >questions won't happen for a few days.
> > >
> > >I tried searching my archives for more information, but had no luck finding
> > >any additional information.
> > >
> > >>>Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> > >>>Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> > >>>Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > >>>---
> > >>> drivers/pci/quirks.c | 21 +++++++++++++++++++++
> > >>> 1 file changed, 21 insertions(+)
> > >>>
> > >>>diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > >>>index 44e0ff3..59fba6e 100644
> > >>>--- a/drivers/pci/quirks.c
> > >>>+++ b/drivers/pci/quirks.c
> > >>>@@ -4431,3 +4431,24 @@ static void quirk_intel_qat_vf_cap(struct
> > >>>pci_dev *pdev)
> > >>> }
> > >>> }
> > >>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> > >>>quirk_intel_qat_vf_cap);
> > >>>+/*
> > >>>+ * Workaround FLR issues for 82579
> > >>>+ * This code disables the FLR (Function Level Reset) via PCIe, in
> > >>>order
> > >>>+ * to workaround a bug found while using device passthrough, where the
> > >>>+ * interface would become non-responsive.
> > >>>+ * NOTE: the FLR bit is Read/Write Once (RWO) in config space, so if
> > >>>+ * the BIOS or kernel writes this register * then this workaround will
> > >>>+ * not work.
> > >>This doesn't sound like a root cause. Is the issue a hardware
> > >>erratum? Linux PCI core bug? VFIO bug? Device firmware bug?
> > >>
> > >>The changelog suggests that the problem only affects passthrough,
> > >>which suggests some sort of kernel bug related to how passthrough is
> > >>implemented.
>
> If this bug affects all scenarios, not just passthrough, the changelog
> should not mention passthrough.
>
> > >>>+ */
> > >>>+static void quirk_intel_flr_cap_dis(struct pci_dev *dev)
> > >>>+{
> > >>>+ int pos = pci_find_capability(dev, PCI_CAP_ID_AF);
> > >>>+ if (pos) {
> > >>>+ u8 cap;
> > >>>+ pci_read_config_byte(dev, pos + PCI_AF_CAP, &cap);
> > >>>+ cap = cap & (~PCI_AF_CAP_FLR);
> > >>>+ pci_write_config_byte(dev, pos + PCI_AF_CAP, cap);
> > >>>+ }
> > >>>+}
> > >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502,
> > >>>quirk_intel_flr_cap_dis);
> > >>>+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503,
> > >>>quirk_intel_flr_cap_dis);
> > >>>--
> > >>>2.7.4
> > >>>
> > >>>--
> > >>>To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > >>>the body of a message to majordomo@vger.kernel.org
> > >>>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > Hello,
> >
> > Original bugzilla thread could be found here:
> > https://bugzilla.redhat.com/show_bug.cgi?format=multiple&id=966840
>
> That bugzilla is private and I can't read it.
Hmm, I can, but I don't see anything in it that supports this. Is that
really the right bz? It's the right hardware, but has all sorts of FUD
about the version of various other components in the stack.
> > This is our HW bug, exist only in 82579 devices. More new devices
> > have no such problem. We have found root cause and suggested this
> > solution.
>
> Is there an erratum you can reference?
>
> > This solution should work for a 95% of cases, so I do not
> > think that this is fragile. For another cases possible solution is
> > get up working system and manually disable FLR, before VM start use
> > our adapter.
>
> I don't think a 95% solution is sufficient. Can you use the
> pci_dev_specific_reset() framework to make a 100% solution?
Right, plus when this does work I suspect it removes the one mechanism
we have to reset the device, which depending on how obscure the failure
scenario is, isn't a clear cut improvement for device assignment.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: R. Parameswaran @ 2016-09-27 19:17 UTC (permalink / raw)
To: David Miller
Cc: parameswaran.r7, kleptog, jchapman, netdev, linux-kernel,
nprachan, rshearma, dfawcus, stephen, acme, lboccass
In-Reply-To: <20160927.033153.1066118008027608891.davem@davemloft.net>
Hi David,
Thanks for the reply, please see inline:
On Tue, 27 Sep 2016, David Miller wrote:
> From: "R. Parameswaran" <parameswaran.r7@gmail.com>
> Date: Thu, 22 Sep 2016 13:52:43 -0700 (PDT)
>
> > From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
> > From: "R. Parameswaran" <rparames@brocade.com>
> > Date: Thu, 22 Sep 2016 13:19:25 -0700
> > Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
> >
> > Take into account all of the tunnel encapsulation headers when setting
> > up the MTU on the L2TP logical interface device. Otherwise, packets
> > created by the applications on top of the L2TP layer are larger
> > than they ought to be, relative to the underlay MTU, leading to
> > needless fragmentation once the outer IP encap is added.
> >
> > Specifically, take into account the (outer, underlay) IP header
> > imposed on the encapsulated L2TP packet, and the Layer 2 header
> > imposed on the inner IP packet prior to L2TP encapsulation.
> >
> > Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
> > and the dst entry in the L2TP tunnel socket to directly pull up
> > the underlay MTU (as the baseline number on top of which the
> > encapsulation headers are factored in). Fall back to Ethernet MTU
> > if this fails.
> >
> > Signed-off-by: R. Parameswaran <rparames@brocade.com>
> >
> > Reviewed-by: "N. Prachanda" <nprachan@brocade.com>,
> > Reviewed-by: "R. Shearman" <rshearma@brocade.com>,
> > Reviewed-by: "D. Fawcus" <dfawcus@brocade.com>
>
> I have to ask, how do other tunnels over UDP such as VXLAN handle
> this problem?
>
Specific to Vxlan, it appears to behave similarly. I haven't functionally
tested fragmentation on vxlan interfaces, but looking at the
code, it seems to account for the headers involved:
When the vxlan interface is created, from vxlan_dev_create(), in
vxlan_setup(), it initially starts off with an ethernet MTU:
vxlan_setup(struct net_device *dev)
{
...
...
ether_setup(dev); <<<<<<< Will set device MTU to 1500
Later, in vxlan_dev_configure(), called from vxlan_dev_create(), it gets
adjusted to account for the headers:
vxlan_dev_configure():
...
if (!conf->mtu)
dev->mtu = lowerdev->mtu - (use_ipv6 ?
VXLAN6_HEADROOM : VXLAN_HEADROOM);
where VXLAN_HEADROOM is defined as follows:
/* IP header + UDP + VXLAN + Ethernet header */
#define VXLAN_HEADROOM (20 + 8 + 8 + 14)
/* IPv6 header + UDP + VXLAN + Ethernet header */
#define VXLAN6_HEADROOM (40 + 8 + 8 + 14)
This seems to match what I see with hand config:
sudo ip link add vxlan0 type vxlan id 42 group 239.1.1.1 dev eth0 dstport
4789 <<<< (eth0 has an MTU of 1500)
sudo ip -d link show vxlan0
36: vxlan0: <BROADCAST,MULTICAST> mtu 1450 qdisc noop state DOWN mode
DEFAULT group default <<<< (1450 = 1500 -50)
link/ether e2:b8:2d:f4:f7:ae brd ff:ff:ff:ff:ff:ff promiscuity 0
vxlan id 42 group 239.1.1.1 dev eth0 srcport 32768 61000 dstport 4789
ageing 300
thanks,
Ramkumar
^ permalink raw reply
* Re: [PATCH net] net: skbuff: Fix incorrect skb->mac_len adjustment in skb_vlan_push()
From: pravin shelar @ 2016-09-27 20:04 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: David S . Miller, Jiri Pirko, Daniel Borkmann,
Linux Kernel Network Developers
In-Reply-To: <1474997505-5059-1-git-send-email-shmulik.ladkani@gmail.com>
On Tue, Sep 27, 2016 at 10:31 AM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> In case 'skb_vlan_push' is called on an skb with a hw-accel vlan tag
> already present, the existing hw-accel tag is inserted into payload, and
> the new given tag is placed as new hw-accel tag.
>
> After the insertion:
> - 'mac_header' is adjusted to point to the new start of the vlan_ethhdr
> - 'data' is adjusted to point to the vlan_hdr portion
> (since packet's payload is the inner 802.1q)
>
> However, 'mac_len' is incorrectly incremented with additional VLAN_HLEN
> bytes, resulting in a total value of 18 bytes.
>
> Meaning, when issuing 'skb_push(skb, skb->mac_len)' the data points
> to random content, 4 bytes PRIOR the ethhdr location.
>
> This is problematic, as many constructs in the stack are issuing
> 'skb_push(skb, skb->mac_len)' prior xmit to a device (e.g tcf_mirred,
> tcf_bpf, nf_dup_netdev_egress), resulting in bogus frames being
> xmitted (having random 4 bytes at start of frame).
>
> For example:
>
> # ip l add dev d0 type dummy
> # tc filter add dev eth0 parent ffff: pref 1 basic \
> action vlan push protocol 802.1ad id 5 pipe \
> action mirred egress redirect dev d0
>
> Any 802.1q (hw-accel) tagged frames arriving on eth0 are xmitted as
> bogus frames on d0; whereas the expected behavior is having QinQ frames.
>
> Fix, removing the unnecessary VLAN_HLEN adjustment of mac_len.
>
> Fixes: 93515d53b1 ("net: move vlan pop/push functions into common code")
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> Cc: Pravin Shelar <pshelar@ovn.org>
> Cc: Jiri Pirko <jiri@mellanox.com>
> ---
>
> - David, if patch ok, suggest this goes to -stable
>
> - Pravin, original push_vlan() code in openvswitch/actions.c prior Jiri
> has moved it into skbuff.c had the following comment:
> /* Update mac_len for subsequent MPLS actions */
> skb->mac_len += VLAN_HLEN;
> Can you please acknowlegde OvS code is also ok with suggested change?
>
OVS MPLS does depends on mac-len to track MPLS header (ref
skb_mpls_header()). Therefore vlan header changes needs to update
mac-len. But after commit 48d2ab609b6bb ("net: mpls: Fixups for GSO")
it does not need to use mac-len to track MPLS header.
To keep simple fix for stable, we can move the skb mac-len adjustment
to OVS module. Later on patch can remove this MPLS dependency on
mac-len for net-next branch.
^ permalink raw reply
* [PATCH net] tg3: Avoid NULL pointer dereference in tg3_io_error_detected()
From: Guilherme G. Piccoli @ 2016-09-27 20:05 UTC (permalink / raw)
To: siva.kallam, prashant, mchan; +Cc: netdev, gpiccoli, Milton Miller
From: Milton Miller <miltonm@us.ibm.com>
While the driver is probing the adapter, an error may occur before the
netdev structure is allocated and attached to pci_dev. In this case,
not only netdev isn't available, but the tg3 private structure is also
not available as it is just math from the NULL pointer, so dereferences
must be skipped.
The following trace is seen when the error is triggered:
[1.402247] Unable to handle kernel paging request for data at address 0x00001a99
[1.402410] Faulting instruction address: 0xc0000000007e33f8
[1.402450] Oops: Kernel access of bad area, sig: 11 [#1]
[1.402481] SMP NR_CPUS=2048 NUMA PowerNV
[1.402513] Modules linked in:
[1.402545] CPU: 0 PID: 651 Comm: eehd Not tainted 4.4.0-36-generic #55-Ubuntu
[1.402591] task: c000001fe4e42a20 ti: c000001fe4e88000 task.ti: c000001fe4e88000
[1.402742] NIP: c0000000007e33f8 LR: c0000000007e3164 CTR: c000000000595ea0
[1.402787] REGS: c000001fe4e8b790 TRAP: 0300 Not tainted (4.4.0-36-generic)
[1.402832] MSR: 9000000100009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 28000422 XER: 20000000
[1.403058] CFAR: c000000000008468 DAR: 0000000000001a99 DSISR: 42000000 SOFTE: 1
GPR00: c0000000007e3164 c000001fe4e8ba10 c0000000015c5e00 0000000000000000
GPR04: 0000000000000001 0000000000000000 0000000000000039 0000000000000299
GPR08: 0000000000000000 0000000000000001 c000001fe4e88000 0000000000000006
GPR12: 0000000000000000 c00000000fb40000 c0000000000e6558 c000003ca1bffd00
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000000d52768
GPR24: c000000000d52740 0000000000000100 c000003ca1b52000 0000000000000002
GPR28: 0000000000000900 0000000000000000 c00000000152a0c0 c000003ca1b52000
[1.404226] NIP [c0000000007e33f8] tg3_io_error_detected+0x308/0x340
[1.404265] LR [c0000000007e3164] tg3_io_error_detected+0x74/0x340
This patch avoids the NULL pointer dereference by moving the access after
the netdev NULL pointer check on tg3_io_error_detected().
Fixes: 0486a063b1ff ("tg3: prevent ifup/ifdown during PCI error recovery")
Fixes: dfc8f370316b ("net/tg3: Release IRQs on permanent error")
Tested-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
Signed-off-by: Milton Miller <miltonm@us.ibm.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
drivers/net/ethernet/broadcom/tg3.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index a2551bc..3a5fce7 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -18122,14 +18122,14 @@ static pci_ers_result_t tg3_io_error_detected(struct pci_dev *pdev,
rtnl_lock();
- /* We needn't recover from permanent error */
- if (state == pci_channel_io_frozen)
- tp->pcierr_recovery = true;
-
/* We probably don't have netdev yet */
if (!netdev || !netif_running(netdev))
goto done;
+ /* We needn't recover from permanent error */
+ if (state == pci_channel_io_frozen)
+ tp->pcierr_recovery = true;
+
tg3_phy_stop(tp);
tg3_netif_stop(tp);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net] tg3: Avoid NULL pointer dereference in tg3_io_error_detected()
From: Michael Chan @ 2016-09-27 20:58 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan, Netdev,
Milton Miller
In-Reply-To: <1475006728-15307-1-git-send-email-gpiccoli@linux.vnet.ibm.com>
On Tue, Sep 27, 2016 at 1:05 PM, Guilherme G. Piccoli
<gpiccoli@linux.vnet.ibm.com> wrote:
> From: Milton Miller <miltonm@us.ibm.com>
>
> While the driver is probing the adapter, an error may occur before the
> netdev structure is allocated and attached to pci_dev. In this case,
> not only netdev isn't available, but the tg3 private structure is also
> not available as it is just math from the NULL pointer, so dereferences
> must be skipped.
>
> The following trace is seen when the error is triggered:
>
> [1.402247] Unable to handle kernel paging request for data at address 0x00001a99
> [1.402410] Faulting instruction address: 0xc0000000007e33f8
> [1.402450] Oops: Kernel access of bad area, sig: 11 [#1]
> [1.402481] SMP NR_CPUS=2048 NUMA PowerNV
> [1.402513] Modules linked in:
> [1.402545] CPU: 0 PID: 651 Comm: eehd Not tainted 4.4.0-36-generic #55-Ubuntu
> [1.402591] task: c000001fe4e42a20 ti: c000001fe4e88000 task.ti: c000001fe4e88000
> [1.402742] NIP: c0000000007e33f8 LR: c0000000007e3164 CTR: c000000000595ea0
> [1.402787] REGS: c000001fe4e8b790 TRAP: 0300 Not tainted (4.4.0-36-generic)
> [1.402832] MSR: 9000000100009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 28000422 XER: 20000000
> [1.403058] CFAR: c000000000008468 DAR: 0000000000001a99 DSISR: 42000000 SOFTE: 1
> GPR00: c0000000007e3164 c000001fe4e8ba10 c0000000015c5e00 0000000000000000
> GPR04: 0000000000000001 0000000000000000 0000000000000039 0000000000000299
> GPR08: 0000000000000000 0000000000000001 c000001fe4e88000 0000000000000006
> GPR12: 0000000000000000 c00000000fb40000 c0000000000e6558 c000003ca1bffd00
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000000d52768
> GPR24: c000000000d52740 0000000000000100 c000003ca1b52000 0000000000000002
> GPR28: 0000000000000900 0000000000000000 c00000000152a0c0 c000003ca1b52000
> [1.404226] NIP [c0000000007e33f8] tg3_io_error_detected+0x308/0x340
> [1.404265] LR [c0000000007e3164] tg3_io_error_detected+0x74/0x340
>
> This patch avoids the NULL pointer dereference by moving the access after
> the netdev NULL pointer check on tg3_io_error_detected().
>
> Fixes: 0486a063b1ff ("tg3: prevent ifup/ifdown during PCI error recovery")
> Fixes: dfc8f370316b ("net/tg3: Release IRQs on permanent error")
> Tested-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
Looks good. Do we need to add !netdev check in tg3_io_resume()?
^ permalink raw reply
* [PATCH v2 net-next 0/4] act_mirred: Ingress actions support
From: Shmulik Ladkani @ 2016-09-27 20:59 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
Florian Westphal, netdev, Shmulik Ladkani
This patch series implements action mirred 'ingress' actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.
v2:
in 1/4, declare tcfm_mac_header_xmit as bool instead of int
Shmulik Ladkani (4):
net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and
make it a bool
net/sched: act_mirred: Refactor detection whether dev needs xmit at
mac header
net/sched: tc_mirred: Rename public predicates
'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
net/sched: act_mirred: Implement ingress actions
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +-
.../net/ethernet/netronome/nfp/nfp_net_offload.c | 2 +-
include/net/tc_act/tc_mirred.h | 6 +-
net/sched/act_mirred.c | 81 ++++++++++++++++------
7 files changed, 70 insertions(+), 29 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and make it a bool
From: Shmulik Ladkani @ 2016-09-27 20:59 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
Florian Westphal, netdev, Shmulik Ladkani
In-Reply-To: <1475009975-30332-1-git-send-email-shmulik.ladkani@gmail.com>
'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
egress to the target device (if action is performed at ingress).
Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
the target device (and use a bool instead of int).
This allows to decouple the attribute from the action to be taken.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
v2: declare tcfm_mac_header_xmit as bool instead of int
include/net/tc_act/tc_mirred.h | 2 +-
net/sched/act_mirred.c | 11 ++++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 62770add15..95431092c4 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -8,7 +8,7 @@ struct tcf_mirred {
struct tc_action common;
int tcfm_eaction;
int tcfm_ifindex;
- int tcfm_ok_push;
+ bool tcfm_mac_header_xmit;
struct net_device __rcu *tcfm_dev;
struct list_head tcfm_list;
};
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 667dc382df..16e17a887b 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -60,11 +60,12 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
struct nlattr *tb[TCA_MIRRED_MAX + 1];
+ bool mac_header_xmit = false;
struct tc_mirred *parm;
struct tcf_mirred *m;
struct net_device *dev;
- int ret, ok_push = 0;
bool exists = false;
+ int ret;
if (nla == NULL)
return -EINVAL;
@@ -102,10 +103,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
case ARPHRD_IPGRE:
case ARPHRD_VOID:
case ARPHRD_NONE:
- ok_push = 0;
+ mac_header_xmit = false;
break;
default:
- ok_push = 1;
+ mac_header_xmit = true;
break;
}
} else {
@@ -136,7 +137,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
dev_hold(dev);
rcu_assign_pointer(m->tcfm_dev, dev);
- m->tcfm_ok_push = ok_push;
+ m->tcfm_mac_header_xmit = mac_header_xmit;
}
if (ret == ACT_P_CREATED) {
@@ -181,7 +182,7 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
goto out;
if (!(at & AT_EGRESS)) {
- if (m->tcfm_ok_push)
+ if (m->tcfm_mac_header_xmit)
skb_push_rcsum(skb2, skb->mac_len);
}
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net-next 2/4] net/sched: act_mirred: Refactor detection whether dev needs xmit at mac header
From: Shmulik Ladkani @ 2016-09-27 20:59 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
Florian Westphal, netdev, Shmulik Ladkani
In-Reply-To: <1475009975-30332-1-git-send-email-shmulik.ladkani@gmail.com>
Move detection logic that tests whether device expects skb data to point
at mac_header upon xmit into a function.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
net/sched/act_mirred.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 16e17a887b..69dcce8c75 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -54,6 +54,20 @@ static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = {
static int mirred_net_id;
static struct tc_action_ops act_mirred_ops;
+static bool dev_is_mac_header_xmit(const struct net_device *dev)
+{
+ switch (dev->type) {
+ case ARPHRD_TUNNEL:
+ case ARPHRD_TUNNEL6:
+ case ARPHRD_SIT:
+ case ARPHRD_IPGRE:
+ case ARPHRD_VOID:
+ case ARPHRD_NONE:
+ return false;
+ }
+ return true;
+}
+
static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
int bind)
@@ -96,19 +110,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
tcf_hash_release(*a, bind);
return -ENODEV;
}
- switch (dev->type) {
- case ARPHRD_TUNNEL:
- case ARPHRD_TUNNEL6:
- case ARPHRD_SIT:
- case ARPHRD_IPGRE:
- case ARPHRD_VOID:
- case ARPHRD_NONE:
- mac_header_xmit = false;
- break;
- default:
- mac_header_xmit = true;
- break;
- }
+ mac_header_xmit = dev_is_mac_header_xmit(dev);
} else {
dev = NULL;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net-next 3/4] net/sched: tc_mirred: Rename public predicates 'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
From: Shmulik Ladkani @ 2016-09-27 20:59 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
Florian Westphal, netdev, Shmulik Ladkani, Hariprasad S,
Jeff Kirsher, Saeed Mahameed, Jiri Pirko, Ido Schimmel,
Jakub Kicinski
In-Reply-To: <1475009975-30332-1-git-send-email-shmulik.ladkani@gmail.com>
These accessors are used in various drivers that support tc offloading,
to detect properties of a given 'tc_action'.
'is_tcf_mirred_redirect' tests that the action is TCA_EGRESS_REDIR.
'is_tcf_mirred_mirror' tests that the action is TCA_EGRESS_MIRROR.
As a prep towards supporting INGRESS redir/mirror, rename these
predicates to reflect their true meaning:
s/is_tcf_mirred_redirect/is_tcf_mirred_egress_redirect/
s/is_tcf_mirred_mirror/is_tcf_mirred_egress_mirror/
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Ido Schimmel <idosch@mellanox.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +++-
drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 2 +-
include/net/tc_act/tc_mirred.h | 4 ++--
6 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
index 49d2debb33..52af62e0ec 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
@@ -113,7 +113,7 @@ static int fill_action_fields(struct adapter *adap,
}
/* Re-direct to specified port in hardware. */
- if (is_tcf_mirred_redirect(a)) {
+ if (is_tcf_mirred_egress_redirect(a)) {
struct net_device *n_dev;
unsigned int i, index;
bool found = false;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a244d9a672..784b0b98ab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8410,7 +8410,7 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
}
/* Redirect to a VF or a offloaded macvlan */
- if (is_tcf_mirred_redirect(a)) {
+ if (is_tcf_mirred_egress_redirect(a)) {
int ifindex = tcf_mirred_ifindex(a);
err = handle_redirect_action(adapter, ifindex, queue,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index a350b7171e..957a464489 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -404,7 +404,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
continue;
}
- if (is_tcf_mirred_redirect(a)) {
+ if (is_tcf_mirred_egress_redirect(a)) {
int ifindex = tcf_mirred_ifindex(a);
struct net_device *out_dev;
struct mlx5e_priv *out_priv;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index fd74d1064f..6a4f9c4664 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1237,8 +1237,10 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
tcf_exts_to_list(cls->exts, &actions);
list_for_each_entry(a, &actions, list) {
- if (!is_tcf_mirred_mirror(a) || protocol != htons(ETH_P_ALL))
+ if (!is_tcf_mirred_egress_mirror(a) ||
+ protocol != htons(ETH_P_ALL)) {
return -ENOTSUPP;
+ }
err = mlxsw_sp_port_add_cls_matchall_mirror(mlxsw_sp_port, cls,
a, ingress);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
index 8acfb631a0..cfed40c0e3 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
@@ -128,7 +128,7 @@ nfp_net_bpf_get_act(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
if (is_tcf_gact_shot(a))
return NN_ACT_TC_DROP;
- if (is_tcf_mirred_redirect(a) &&
+ if (is_tcf_mirred_egress_redirect(a) &&
tcf_mirred_ifindex(a) == nn->netdev->ifindex)
return NN_ACT_TC_REDIR;
}
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 95431092c4..604bc31e23 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -14,7 +14,7 @@ struct tcf_mirred {
};
#define to_mirred(a) ((struct tcf_mirred *)a)
-static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_redirect(const struct tc_action *a)
{
#ifdef CONFIG_NET_CLS_ACT
if (a->ops && a->ops->type == TCA_ACT_MIRRED)
@@ -23,7 +23,7 @@ static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
return false;
}
-static inline bool is_tcf_mirred_mirror(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_mirror(const struct tc_action *a)
{
#ifdef CONFIG_NET_CLS_ACT
if (a->ops && a->ops->type == TCA_ACT_MIRRED)
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-09-27 20:59 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
Florian Westphal, netdev, Shmulik Ladkani
In-Reply-To: <1475009975-30332-1-git-send-email-shmulik.ladkani@gmail.com>
Up until now, 'action mirred' supported only egress actions (either
TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
This patch implements the corresponding ingress actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_mirred.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 69dcce8c75..21f0f5f868 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -33,6 +33,25 @@
static LIST_HEAD(mirred_list);
static DEFINE_SPINLOCK(mirred_list_lock);
+static bool tcf_mirred_is_act_redirect(int action)
+{
+ return action == TCA_EGRESS_REDIR || action == TCA_INGRESS_REDIR;
+}
+
+static u32 tcf_mirred_act_direction(int action)
+{
+ switch (action) {
+ case TCA_EGRESS_REDIR:
+ case TCA_EGRESS_MIRROR:
+ return AT_EGRESS;
+ case TCA_INGRESS_REDIR:
+ case TCA_INGRESS_MIRROR:
+ return AT_INGRESS;
+ default:
+ BUG();
+ }
+}
+
static void tcf_mirred_release(struct tc_action *a, int bind)
{
struct tcf_mirred *m = to_mirred(a);
@@ -97,6 +116,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
switch (parm->eaction) {
case TCA_EGRESS_MIRROR:
case TCA_EGRESS_REDIR:
+ case TCA_INGRESS_REDIR:
+ case TCA_INGRESS_MIRROR:
break;
default:
if (exists)
@@ -158,7 +179,8 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
struct tcf_mirred *m = to_mirred(a);
struct net_device *dev;
struct sk_buff *skb2;
- int retval, err;
+ int retval, err = 0;
+ int mac_len;
u32 at;
tcf_lastuse_update(&m->tcf_tm);
@@ -183,23 +205,37 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
if (!skb2)
goto out;
- if (!(at & AT_EGRESS)) {
- if (m->tcfm_mac_header_xmit)
+ /* If action's target direction differs than filter's direction,
+ * and devices expect a mac header on xmit, then mac push/pull is
+ * needed.
+ */
+ if (at != tcf_mirred_act_direction(m->tcfm_eaction) &&
+ m->tcfm_mac_header_xmit) {
+ if (at & AT_EGRESS) {
+ /* caught at egress, act ingress: pull mac */
+ mac_len = skb_network_header(skb) - skb_mac_header(skb);
+ skb_pull_rcsum(skb2, mac_len);
+ } else {
+ /* caught at ingress, act egress: push mac */
skb_push_rcsum(skb2, skb->mac_len);
+ }
}
/* mirror is always swallowed */
- if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+ if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
skb2->skb_iif = skb->dev->ifindex;
skb2->dev = dev;
- err = dev_queue_xmit(skb2);
+ if (tcf_mirred_act_direction(m->tcfm_eaction) & AT_EGRESS)
+ err = dev_queue_xmit(skb2);
+ else
+ netif_receive_skb(skb2);
if (err) {
out:
qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
- if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+ if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
retval = TC_ACT_SHOT;
}
rcu_read_unlock();
--
2.7.4
^ permalink raw reply related
* Re: [v3,net-next] net: phy: Add Edge-rate driver for Microsemi PHYs.
From: Andrew Lunn @ 2016-09-27 21:14 UTC (permalink / raw)
To: Raju.Lakkaraju; +Cc: netdev
I just realised the possibly correct binding was starring me in the
face.
--------------------------------------------------------------|
| 3.3V 2.5V 1.8V 1.5V |
|-------------------------------------------------------------|
|-2% -3% -5% -6% |
|-------------------------------------------------------------|
|-4% -6% -9% -14% |
|-------------------------------------------------------------|
|-7% -10% -16% -21% |
|-------------------------------------------------------------|
|-10% -14% -23% -29% |
|-------------------------------------------------------------|
|-17% -23% -35% -42% |
|-------------------------------------------------------------|
|-29% -37% -52% -58% |
|-------------------------------------------------------------|
|-53% -63% -76% -77% |
|-------------------------------------------------------------|
So the binding is:
vsc8531,vddmac : The vddmac in mV.
vsc8531,edge-slowdown : % the edge should be slowed down relative to
the fastest possible edge time.
Given those two values, the driver can work out the magic value to put
into the register that nobody knows the true meaning of.
Andrew
^ permalink raw reply
* [PATCH net] i40e: avoid NULL pointer dereference and recursive errors on early PCI error
From: Guilherme G. Piccoli @ 2016-09-27 21:14 UTC (permalink / raw)
To: jeffrey.t.kirsher, intel-wired-lan; +Cc: netdev, gpiccoli
Although rare, it's possible to hit PCI error early on device
probe, meaning possibly some structs are not entirely initialized,
and some might even be completely uninitialized, leading to NULL
pointer dereference.
The i40e driver currently presents a "bad" behavior if device hits
such early PCI error: firstly, the struct i40e_pf might not be
attached to pci_dev yet, leading to a NULL pointer dereference on
access to pf->state.
Even checking if the struct is NULL and avoiding the access in that
case isn't enough, since the driver cannot recover from PCI error
that early; in our experiments we saw multiple failures on kernel
log, like:
[549.664] i40e 0007:01:00.1: Initial pf_reset failed: -15
[549.664] i40e: probe of 0007:01:00.1 failed with error -15
[...]
[871.644] i40e 0007:01:00.1: The driver for the device stopped because the
device firmware failed to init. Try updating your NVM image.
[871.644] i40e: probe of 0007:01:00.1 failed with error -32
[...]
[872.516] i40e 0007:01:00.0: ARQ: Unknown event 0x0000 ignored
Between the first probe failure (error -15) and the second (error -32)
another PCI error happened due to the first bad probe. Also, driver
started to flood console with those ARQ event messages.
This patch will prevent these issues by allowing error recovery
mechanism to remove the failed device from the system instead of
trying to recover from early PCI errors during device probe.
Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d0b3a1b..dad15b6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11360,6 +11360,12 @@ static pci_ers_result_t i40e_pci_error_detected(struct pci_dev *pdev,
dev_info(&pdev->dev, "%s: error %d\n", __func__, error);
+ if (!pf) {
+ dev_info(&pdev->dev,
+ "Cannot recover - error happened during device probe\n");
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
/* shutdown all operations */
if (!test_bit(__I40E_SUSPENDED, &pf->state)) {
rtnl_lock();
--
2.1.0
^ permalink raw reply related
* [PATCH v2 net-next] tcp: Change txhash on every SYN and RTO retransmit
From: Lawrence Brakmo @ 2016-09-27 21:23 UTC (permalink / raw)
To: netdev; +Cc: Kernel Team, Eric Dumazet, Yuchung Cheng
The current code changes txhash (flowlables) on every retransmitted
SYN/ACK, but only after the 2nd retransmitted SYN and only after
tcp_retries1 RTO retransmits.
With this patch:
1) txhash is changed with every SYN retransmits
2) adds the option for the txhash to be changed before tcp_retries1
RTO retransmits. A new sysctl tcp_rto_txhash_prob represents the
probability that txhash will be changed. The default value is 0
which maintains previous behavior and a value of 100 will always
change it.
The result is that we can start re-routing around failed (or very
congested paths) as soon as possible. Otherwise application health
checks may fail and the connection may be terminated before we start
to change txhash.
v2: Added sysctl documentation and cleaned code
Tested with packetdrill tests
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
Documentation/networking/ip-sysctl.txt | 11 +++++++++++
include/net/tcp.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 10 ++++++++++
net/ipv4/tcp_input.c | 2 ++
net/ipv4/tcp_timer.c | 4 ++++
5 files changed, 28 insertions(+)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 3db8c67..0e7f9ac 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -472,6 +472,17 @@ tcp_max_reordering - INTEGER
if paths are using per packet load balancing (like bonding rr mode)
Default: 300
+tcp_rto_txhash_prob - INTEGER
+ Probability [0 to 100] that we will recalculate txhash when a
+ packet is resent due to an RTO and the RTO for this packet has
+ fired less than tcp_retries1 times. It is always recalculated
+ after tcp_retries_times.
+
+ Setting it to 100 helps re-route around failed (or very congested
+ paths) as soon as possible. Otherwise application health checks may
+ fail and the connection may be terminated before the txhash has
+ a chance to change.
+
tcp_retrans_collapse - BOOLEAN
Bug-to-bug compatibility with some broken printers.
On retransmit try to send bigger packets to work around bugs in
diff --git a/include/net/tcp.h b/include/net/tcp.h
index f83b7f2..406d474 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -271,6 +271,7 @@ extern int sysctl_tcp_autocorking;
extern int sysctl_tcp_invalid_ratelimit;
extern int sysctl_tcp_pacing_ss_ratio;
extern int sysctl_tcp_pacing_ca_ratio;
+extern int sysctl_tcp_rto_txhash_prob;
extern atomic_long_t tcp_memory_allocated;
extern struct percpu_counter tcp_sockets_allocated;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 1cb67de..0b185a1 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -28,6 +28,7 @@
static int zero;
static int one = 1;
static int four = 4;
+static int hundred = 100;
static int thousand = 1000;
static int gso_max_segs = GSO_MAX_SEGS;
static int tcp_retr1_max = 255;
@@ -624,6 +625,15 @@ static struct ctl_table ipv4_table[] = {
.proc_handler = proc_dointvec_ms_jiffies,
},
{
+ .procname = "tcp_rto_txhash_prob",
+ .data = &sysctl_tcp_rto_txhash_prob,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &hundred,
+ },
+ {
.procname = "icmp_msgs_per_sec",
.data = &sysctl_icmp_msgs_per_sec,
.maxlen = sizeof(int),
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8c6ad2d..2fea29d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -101,6 +101,8 @@ int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
int sysctl_tcp_early_retrans __read_mostly = 3;
int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2;
+int sysctl_tcp_rto_txhash_prob __read_mostly = 100;
+
#define FLAG_DATA 0x01 /* Incoming frame contained data. */
#define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a window update. */
#define FLAG_DATA_ACKED 0x04 /* This ACK acknowledged new data. */
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index f712b41..8bdb215 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -192,6 +192,8 @@ static int tcp_write_timeout(struct sock *sk)
if (tp->syn_data && icsk->icsk_retransmits == 1)
NET_INC_STATS(sock_net(sk),
LINUX_MIB_TCPFASTOPENACTIVEFAIL);
+ } else if (!tp->syn_data && !tp->syn_fastopen) {
+ sk_rethink_txhash(sk);
}
retry_until = icsk->icsk_syn_retries ? : net->ipv4.sysctl_tcp_syn_retries;
syn_set = true;
@@ -213,6 +215,8 @@ static int tcp_write_timeout(struct sock *sk)
tcp_mtu_probing(icsk, sk);
dst_negative_advice(sk);
+ } else if (prandom_u32_max(100) < sysctl_tcp_rto_txhash_prob) {
+ sk_rethink_txhash(sk);
}
retry_until = net->ipv4.sysctl_tcp_retries2;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2 net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Eric Dumazet @ 2016-09-27 21:27 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: David Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
Daniel Borkmann, Florian Westphal, netdev
In-Reply-To: <1475009975-30332-5-git-send-email-shmulik.ladkani@gmail.com>
On Tue, 2016-09-27 at 23:59 +0300, Shmulik Ladkani wrote:
> Up until now, 'action mirred' supported only egress actions (either
> TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
>
> This patch implements the corresponding ingress actions
> TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
> - if (m->tcfm_mac_header_xmit)
> + /* If action's target direction differs than filter's direction,
> + * and devices expect a mac header on xmit, then mac push/pull is
> + * needed.
> + */
> + if (at != tcf_mirred_act_direction(m->tcfm_eaction) &&
Note that m->tcfm_eaction is read here.
> + m->tcfm_mac_header_xmit) {
> + if (at & AT_EGRESS) {
> + /* caught at egress, act ingress: pull mac */
> + mac_len = skb_network_header(skb) - skb_mac_header(skb);
> + skb_pull_rcsum(skb2, mac_len);
> + } else {
> + /* caught at ingress, act egress: push mac */
> skb_push_rcsum(skb2, skb->mac_len);
> + }
> }
>
> /* mirror is always swallowed */
> - if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
> + if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
> skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
>
> skb2->skb_iif = skb->dev->ifindex;
> skb2->dev = dev;
> - err = dev_queue_xmit(skb2);
Note that m->tcfm_eaction is read another time here.
> + if (tcf_mirred_act_direction(m->tcfm_eaction) & AT_EGRESS)
> + err = dev_queue_xmit(skb2);
> + else
> + netif_receive_skb(skb2);
>
Since this runs lockless, another cpu might change m->tcfm_eaction in
the middle, and you could call dev_queue_xmit(skb2) while the skb2 was
prepared for the opposite action.
I guess some drivers could crash, because they expect to find a MAC
header.
If not, a comment would be nice.
Thanks.
^ permalink raw reply
* Re: [PATCH net] tg3: Avoid NULL pointer dereference in tg3_io_error_detected()
From: Guilherme G. Piccoli @ 2016-09-27 21:27 UTC (permalink / raw)
To: Michael Chan
Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan, Netdev,
Milton Miller
In-Reply-To: <CACKFLi=MVovAMnZwTe3Swb1+hgy+h9CfTURPVrZsM0MJGV9Khw@mail.gmail.com>
On 09/27/2016 05:58 PM, Michael Chan wrote:
> On Tue, Sep 27, 2016 at 1:05 PM, Guilherme G. Piccoli
> <gpiccoli@linux.vnet.ibm.com> wrote:
>> From: Milton Miller <miltonm@us.ibm.com>
>>
>> While the driver is probing the adapter, an error may occur before the
>> netdev structure is allocated and attached to pci_dev. In this case,
>> not only netdev isn't available, but the tg3 private structure is also
>> not available as it is just math from the NULL pointer, so dereferences
>> must be skipped.
>>
>> The following trace is seen when the error is triggered:
>>
>> [1.402247] Unable to handle kernel paging request for data at address 0x00001a99
>> [1.402410] Faulting instruction address: 0xc0000000007e33f8
>> [1.402450] Oops: Kernel access of bad area, sig: 11 [#1]
>> [1.402481] SMP NR_CPUS=2048 NUMA PowerNV
>> [1.402513] Modules linked in:
>> [1.402545] CPU: 0 PID: 651 Comm: eehd Not tainted 4.4.0-36-generic #55-Ubuntu
>> [1.402591] task: c000001fe4e42a20 ti: c000001fe4e88000 task.ti: c000001fe4e88000
>> [1.402742] NIP: c0000000007e33f8 LR: c0000000007e3164 CTR: c000000000595ea0
>> [1.402787] REGS: c000001fe4e8b790 TRAP: 0300 Not tainted (4.4.0-36-generic)
>> [1.402832] MSR: 9000000100009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 28000422 XER: 20000000
>> [1.403058] CFAR: c000000000008468 DAR: 0000000000001a99 DSISR: 42000000 SOFTE: 1
>> GPR00: c0000000007e3164 c000001fe4e8ba10 c0000000015c5e00 0000000000000000
>> GPR04: 0000000000000001 0000000000000000 0000000000000039 0000000000000299
>> GPR08: 0000000000000000 0000000000000001 c000001fe4e88000 0000000000000006
>> GPR12: 0000000000000000 c00000000fb40000 c0000000000e6558 c000003ca1bffd00
>> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000000d52768
>> GPR24: c000000000d52740 0000000000000100 c000003ca1b52000 0000000000000002
>> GPR28: 0000000000000900 0000000000000000 c00000000152a0c0 c000003ca1b52000
>> [1.404226] NIP [c0000000007e33f8] tg3_io_error_detected+0x308/0x340
>> [1.404265] LR [c0000000007e3164] tg3_io_error_detected+0x74/0x340
>>
>> This patch avoids the NULL pointer dereference by moving the access after
>> the netdev NULL pointer check on tg3_io_error_detected().
>>
>> Fixes: 0486a063b1ff ("tg3: prevent ifup/ifdown during PCI error recovery")
>> Fixes: dfc8f370316b ("net/tg3: Release IRQs on permanent error")
>> Tested-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
>> Signed-off-by: Milton Miller <miltonm@us.ibm.com>
>> Signed-off-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
>
> Looks good. Do we need to add !netdev check in tg3_io_resume()?
Thanks Michael. It's a good point - I didn't trigger any error without
the check, but looking at error handlers, every one seems to have this
check except tg3_io_resume().
Do you want us to send a v2 including this check? Or maybe another patch?
Cheers,
Guilherme
^ 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