* [PATCH net-next] Revert "ipv6: Initial skb->dev and skb->protocol in ip6_output"
From: Chenbo Feng @ 2017-06-09 19:56 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Eric Dumazet, Chenbo Feng
From: Chenbo Feng <fengc@google.com>
This reverts commit 97a7a37a7b7b("ipv6: Initial skb->dev and
skb->protocol in ip6_output") since it does not handles the
skb->dev assignment inside ip6_fragment() code path properly.
Need to rework and upload again
Fixes: 97a7a37a7b7b("ipv6: Initial skb->dev and skb->protocol in ip6_output")
Signed-off-by: Chenbo Feng <fengc@google.com>
---
net/ipv6/ip6_output.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 02cd44f..bf8a58a 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -67,6 +67,9 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
struct in6_addr *nexthop;
int ret;
+ skb->protocol = htons(ETH_P_IPV6);
+ skb->dev = dev;
+
if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) {
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
@@ -151,9 +154,6 @@ int ip6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
struct net_device *dev = skb_dst(skb)->dev;
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
- skb->protocol = htons(ETH_P_IPV6);
- skb->dev = dev;
-
if (unlikely(idev->cnf.disable_ipv6)) {
IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
kfree_skb(skb);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] cxgb4: fix memory leak in init_one()
From: David Miller @ 2017-06-09 19:53 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh
In-Reply-To: <1497016584-12034-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Fri, 9 Jun 2017 19:26:24 +0530
> Free up mbox_log allocated for PF0 to PF3.
>
> Fixes: 7829451c695e ("cxgb4: Add control net_device for configuring PCIe VF")
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] qed: add qed_int_sb_init() stub function
From: David Miller @ 2017-06-09 19:52 UTC (permalink / raw)
To: arnd
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, Manish.Chopra, netdev,
linux-kernel
In-Reply-To: <20170609103744.3239316-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 9 Jun 2017 12:37:35 +0200
> When CONFIG_QED_SRIOV is disabled, we get a build error:
>
> drivers/net/ethernet/qlogic/qed/qed_int.c: In function 'qed_int_sb_init':
> drivers/net/ethernet/qlogic/qed/qed_int.c:1499:4: error: implicit declaration of function 'qed_vf_set_sb_info'; did you mean 'qed_mcp_get_resc_info'? [-Werror=implicit-function-declaration]
>
> All the other declarations have a 'static inline' stub as an alternative
> here, so this adds one more for qed_int_sb_init.
>
> Fixes: 50a207147fce ("qed: Hold a single array for SBs")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v2 7/8] net: mvmdio: add xmdio support
From: Russell King - ARM Linux @ 2017-06-09 19:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, Florian Fainelli, davem, jason, gregory.clement,
sebastian.hesselbarth, thomas.petazzoni, mw, netdev,
linux-arm-kernel
In-Reply-To: <20170609144936.GF20756@lunn.ch>
On Fri, Jun 09, 2017 at 04:49:36PM +0200, Andrew Lunn wrote:
> On Fri, Jun 09, 2017 at 04:09:22PM +0200, Antoine Tenart wrote:
> > The MDIO/xMDIO registers are embedded into the network controller. The
> > mvmdio driver was created at first to abstract this functionality
> > outside the network controller driver because it is shared between all
> > ports and used in different IPs. So it's not really devices per say.
> >
> > Looking at the datasheet/schematics there are two hardware buses, one
> > for c22 and one for c45. So we should keep two separate nodes to
> > describe the two interfaces. From what I read c45 is backward
> > compatible with c22 so the xSMI interface should be capable to speak to
> > c22 PHYs as well.
>
> The on the wire protocol of c45 is backwards compatible with c22, in
> that a c22 device will not get confused by a c45 transaction on the
> bus. A c22 device will just ignore it. You cannot talk to a c22 device
> using c45.
>From what I can tell, having 'scoped the MDIO line and tried writing
several different values to the XSMI registers, it is not possible
for the XSMI block to generate C22 frame structures - the "start"
bits are always "00", and I can't make them the required "01" for C22.
However, I can confirm that bits 26 and 27 of the XSMI register are used
directly for the OP field (so 0 << 26 produces a C45 address frame.) I
suspect, although I haven't delved that deeply (yet) that bit 28 sets
whether XSMI produces an address cycle itself along with the data cycle.
Bit 31 appears to be writable, but has no effect on the frame structure.
> What i'm worried about is there being one set of MDC/MDIO lines. You
> should not expose that to linux as two mdio busses. It is one bus.
We're independent - the SMI and XSMI blocks are two entirely separate
interfaces with entirely separate hardware MDC/MDIO lines. The XSMI
MDC/MDIO lines remain at logic '1' while phylib polls the PHY via
the SMI interface.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/8] qed*: Light L2 updates
From: David Miller @ 2017-06-09 19:49 UTC (permalink / raw)
To: Yuval.Mintz; +Cc: netdev, linux-rdma, Michal.Kalderon
In-Reply-To: <20170609141325.20547-1-Yuval.Mintz@cavium.com>
From: Yuval Mintz <Yuval.Mintz@cavium.com>
Date: Fri, 9 Jun 2017 17:13:17 +0300
> This series does a major overhaul of the LL2 logic in qed.
> The single biggest change done here is in #5 where we're changing
> the API qed provides for LL2 [both internally in case of storage and
> externally in case of RoCE] to become callback-based to allow cleaner
> scalability in preperation to the future iWARP submission which would
> aadd additional flavors of LL2. It's also the only patch in series
> to modify !qed logic [qedr].
>
> Patches prior to that mostly deal with refactoring LL2 code,
> encapsulating varaious parameters into structure and re-ordering
> of LL2 code. The latter patches add some small missing bits of LL2
> ffunctionality.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng @ 2017-06-09 19:45 UTC (permalink / raw)
To: Bjørn Mork; +Cc: David Miller, netdev, lorenzo, edumazet, fengc
In-Reply-To: <87d1adge1i.fsf@miraculix.mork.no>
On 06/09/2017 12:24 PM, Bjørn Mork wrote:
> Chenbo Feng <chenbofeng.kernel@gmail.com> writes:
>
>> This patch is still under working since it may have problem with
>> ip_fragment() call, did you applied it already? Should I send a revert
>> patch to you then?
> It does? I initially thought so too, but looking closer I believe the
> ip6_copy_metadata() calls in ip6_fragment() takes care of it.
>
>
>
> Bjørn
>
At least in the fail_toobig code path of ip_fragment() call, skb->dev
get assigned again. It seems to be redundant with this patch or it will
rewrite the skb->dev field. I will revert this one and upload again
after I have a proper handle for that.
^ permalink raw reply
* Re: pull-request: can 2017-06-09
From: David Miller @ 2017-06-09 19:42 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <20170609125513.24436-1-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Fri, 9 Jun 2017 14:55:07 +0200
> this is a pull request of 6 patches for net/master.
>
> There's a patch by Stephane Grosjean that fixes an uninitialized symbol warning
> in the peak_canfd driver. A patch by Johan Hovold to fix the product-id
> endianness in an error message in the the peak_usb driver. A patch by Oliver
> Hartkopp to enable CAN FD for virtual CAN devices by default. Three patches by
> me, one makes the helper function can_change_state() robust to be called with
> cf == NULL. The next patch fixes a memory leak in the gs_usb driver. And the
> last one fixes a lockdep splat by properly initialize the per-net
> can_rcvlists_lock spin_lock.
Pulled, thank you Marc.
^ permalink raw reply
* Re: [PATCH net] mac80211: free netdev on dev_alloc_name() error
From: David Miller @ 2017-06-09 19:40 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, netdev, johannes.berg
In-Reply-To: <20170609193309.14972-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 9 Jun 2017 21:33:09 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> The change to remove free_netdev() from ieee80211_if_free()
> erroneously didn't add the necessary free_netdev() for when
> ieee80211_if_free() is called directly in one place, rather
> than as the priv_destructor. Add the missing call.
>
> Fixes: cf124db566e6 ("net: Fix inconsistent teardown and release of private netdev state.")
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Applied, thanks Johannes.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: David Miller @ 2017-06-09 19:39 UTC (permalink / raw)
To: chenbofeng.kernel; +Cc: netdev, lorenzo, edumazet, fengc
In-Reply-To: <3270a030-2608-1629-07d6-7fdeee9e98cc@gmail.com>
From: Chenbo Feng <chenbofeng.kernel@gmail.com>
Date: Fri, 9 Jun 2017 12:13:57 -0700
>
>
> On 06/09/2017 12:08 PM, David Miller wrote:
>> From: Chenbo Feng <chenbofeng.kernel@gmail.com>
>> Date: Fri, 9 Jun 2017 12:06:07 -0700
>>
>>> From: Chenbo Feng <fengc@google.com>
>>>
>>> Move the initialization of skb->dev and skb->protocol from
>>> ip6_finish_output2 to ip6_output. This can make the skb->dev and
>>> skb->protocol information avalaible to the CGROUP eBPF filter.
>>>
>>> Signed-off-by: Chenbo Feng <fengc@google.com>
>>> Acked-by: Eric Dumazet <edumazet@google.com>
>> Applied, thanks.
>>
>> This makes ipv6 consistent with ipv4.
>>
>> I am surprised this wasn't noticed, for example, in netfilter.
>> .
>>
> Hi David,
>
> This patch is still under working since it may have problem with
> ip_fragment() call, did you applied it already? Should I send a revert
> patch to you then?
A revert is necessary or a relative fixup.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: David Miller @ 2017-06-09 19:39 UTC (permalink / raw)
To: fengc; +Cc: chenbofeng.kernel, netdev, lorenzo, edumazet
In-Reply-To: <CAMOXUJnJ+e=f3x7gZsh4f=Fn01pspDqMKJFqc7cb7umV8SXtaw@mail.gmail.com>
From: Chenbo Feng <fengc@google.com>
Date: Fri, 9 Jun 2017 12:08:39 -0700
> Sorry, this is the wrong patch, please ignore it.
:-/ already applied it.
You must now send a relative fixup patch.
^ permalink raw reply
* Re: [PATCH net-next 00/11] r8152: minor adjustment
From: David Miller @ 2017-06-09 19:38 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-260-Taiwan-albertk@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Fri, 9 Jun 2017 17:11:37 +0800
> Adjust some code to make it reasonable or satisfy the suggestion from
> the engineers.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net RESEND] net: rps: send out pending IPI's on CPU hotplug
From: David Miller @ 2017-06-09 19:35 UTC (permalink / raw)
To: ashwanth; +Cc: netdev, eric.dumazet
In-Reply-To: <1496998498-20836-1-git-send-email-ashwanth@codeaurora.org>
From: Ashwanth Goli <ashwanth@codeaurora.org>
Date: Fri, 9 Jun 2017 14:24:58 +0530
> IPI's from the victim cpu are not handled in dev_cpu_callback.
> So these pending IPI's would be sent to the remote cpu only when
> NET_RX is scheduled on the victim cpu and since this trigger is
> unpredictable it would result in packet latencies on the remote cpu.
>
> This patch add support to send the pending ipi's of victim cpu.
>
> Signed-off-by: Ashwanth Goli <ashwanth@codeaurora.org>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next 0/8] Bug fixes in ena ethernet driver
From: David Miller @ 2017-06-09 19:33 UTC (permalink / raw)
To: netanel; +Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, evgenys
In-Reply-To: <1496991325-551-1-git-send-email-netanel@amazon.com>
From: <netanel@amazon.com>
Date: Fri, 9 Jun 2017 09:55:16 +0300
> This patchset contains fixes for the bugs that were discovered so far.
You submitted patch #6 twice, once with the word "stuck" in the subject
line, once with the word "hang" in the subject line.
Please sort this out and resubmit, thanks.
^ permalink raw reply
* [PATCH net] mac80211: free netdev on dev_alloc_name() error
From: Johannes Berg @ 2017-06-09 19:33 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
The change to remove free_netdev() from ieee80211_if_free()
erroneously didn't add the necessary free_netdev() for when
ieee80211_if_free() is called directly in one place, rather
than as the priv_destructor. Add the missing call.
Fixes: cf124db566e6 ("net: Fix inconsistent teardown and release of private netdev state.")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/iface.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 915d7e1b4545..f5f50150ba1c 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1816,6 +1816,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
ret = dev_alloc_name(ndev, ndev->name);
if (ret < 0) {
ieee80211_if_free(ndev);
+ free_netdev(ndev);
return ret;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Bjørn Mork @ 2017-06-09 19:24 UTC (permalink / raw)
To: Chenbo Feng; +Cc: David Miller, netdev, lorenzo, edumazet, fengc
In-Reply-To: <3270a030-2608-1629-07d6-7fdeee9e98cc@gmail.com>
Chenbo Feng <chenbofeng.kernel@gmail.com> writes:
> This patch is still under working since it may have problem with
> ip_fragment() call, did you applied it already? Should I send a revert
> patch to you then?
It does? I initially thought so too, but looking closer I believe the
ip6_copy_metadata() calls in ip6_fragment() takes care of it.
Bjørn
^ permalink raw reply
* [PATCH net-next] bpf: Remove duplicate tcp_filter hook in ipv6
From: Chenbo Feng @ 2017-06-09 19:17 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Eric Dumazet, Chenbo Feng
From: Chenbo Feng <fengc@google.com>
There are two tcp_filter hooks in tcp_ipv6 ingress path currently.
One is at tcp_v6_rcv and another is in tcp_v6_do_rcv. It seems the
tcp_filter() call inside tcp_v6_do_rcv is redundent and some packet
will be filtered twice in this situation. This will cause trouble
when using eBPF filters to account traffic data.
Signed-off-by: Chenbo Feng <fengc@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/tcp_ipv6.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 0840543..84ad502 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1249,9 +1249,6 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
if (skb->protocol == htons(ETH_P_IP))
return tcp_v4_do_rcv(sk, skb);
- if (tcp_filter(sk, skb))
- goto discard;
-
/*
* socket locking is here for SMP purposes as backlog rcv
* is currently called with bh processing disabled.
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 2/2] bpf: Fix test_obj_id.c for llvm 5.0
From: David Miller @ 2017-06-09 19:15 UTC (permalink / raw)
To: kafai; +Cc: netdev, ast, daniel, yhs, kernel-team
In-Reply-To: <20170609053017.2909855-2-kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Thu, 8 Jun 2017 22:30:17 -0700
> llvm 5.0 does not like the section name and the function name
> to be the same:
>
> clang -I. -I./include/uapi -I../../../include/uapi \
> -I../../../../samples/bpf/ \
> -Wno-compare-distinct-pointer-types \
> -O2 -target bpf -c \
> linux/tools/testing/selftests/bpf/test_obj_id.c -o \
> linux/tools/testing/selftests/bpf/test_obj_id.o
> fatal error: error in backend: 'test_prog_id' label emitted multiple times to
> assembly file
> clang-5.0: error: clang frontend command failed with exit code 70 (use -v to
> see invocation)
> clang version 5.0.0 (trunk 304326) (llvm/trunk 304329)
>
> This patch makes changes to the section name and the function name.
>
> Fixes: 95b9afd3987f ("bpf: Test for bpf ID")
> Reported-by: Alexei Starovoitov <ast@fb.com>
> Reported-by: Yonghong Song <yhs@fb.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 1/2] bpf: Fix test_bpf_obj_id() when the bpf_jit_enable sysctl is diabled
From: David Miller @ 2017-06-09 19:15 UTC (permalink / raw)
To: kafai; +Cc: netdev, ast, daniel, yhs, kernel-team
In-Reply-To: <20170609053017.2909855-1-kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Thu, 8 Jun 2017 22:30:16 -0700
> test_bpf_obj_id() should not expect a non zero jited_prog_len
> to be returned by bpf_obj_get_info_by_fd() when
> net.core.bpf_jit_enable is 0.
>
> The patch checks for net.core.bpf_jit_enable and
> has different expectation on jited_prog_len.
>
> This patch also removes the pwd.h header which I forgot
> to remove after making changes.
>
> Fixes: 95b9afd3987f ("bpf: Test for bpf ID")
> Reported-by: Yonghong Song <yhs@fb.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Applied, but please in the future provide a proper header posting
with Subject "[PATCH net-next 0/N] ...".
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng @ 2017-06-09 19:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, lorenzo, edumazet, fengc
In-Reply-To: <20170609.150836.1427149901008136163.davem@davemloft.net>
On 06/09/2017 12:08 PM, David Miller wrote:
> From: Chenbo Feng <chenbofeng.kernel@gmail.com>
> Date: Fri, 9 Jun 2017 12:06:07 -0700
>
>> From: Chenbo Feng <fengc@google.com>
>>
>> Move the initialization of skb->dev and skb->protocol from
>> ip6_finish_output2 to ip6_output. This can make the skb->dev and
>> skb->protocol information avalaible to the CGROUP eBPF filter.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> Acked-by: Eric Dumazet <edumazet@google.com>
> Applied, thanks.
>
> This makes ipv6 consistent with ipv4.
>
> I am surprised this wasn't noticed, for example, in netfilter.
> .
>
Hi David,
This patch is still under working since it may have problem with
ip_fragment() call, did you applied it already? Should I send a revert
patch to you then?
Chenbo Feng
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: David Miller @ 2017-06-09 19:08 UTC (permalink / raw)
To: chenbofeng.kernel; +Cc: netdev, lorenzo, edumazet, fengc
In-Reply-To: <1497035168-9093-1-git-send-email-chenbofeng.kernel@gmail.com>
From: Chenbo Feng <chenbofeng.kernel@gmail.com>
Date: Fri, 9 Jun 2017 12:06:07 -0700
> From: Chenbo Feng <fengc@google.com>
>
> Move the initialization of skb->dev and skb->protocol from
> ip6_finish_output2 to ip6_output. This can make the skb->dev and
> skb->protocol information avalaible to the CGROUP eBPF filter.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
This makes ipv6 consistent with ipv4.
I am surprised this wasn't noticed, for example, in netfilter.
^ permalink raw reply
* [PATCH net-next] ipv6: Initial skb->dev and skb->protocol in ip6_output
From: Chenbo Feng @ 2017-06-09 19:06 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Lorenzo Colitti, Eric Dumazet, Chenbo Feng
From: Chenbo Feng <fengc@google.com>
Move the initialization of skb->dev and skb->protocol from
ip6_finish_output2 to ip6_output. This can make the skb->dev and
skb->protocol information avalaible to the CGROUP eBPF filter.
Signed-off-by: Chenbo Feng <fengc@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/ip6_output.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index bf8a58a..02cd44f 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -67,9 +67,6 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
struct in6_addr *nexthop;
int ret;
- skb->protocol = htons(ETH_P_IPV6);
- skb->dev = dev;
-
if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) {
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
@@ -154,6 +151,9 @@ int ip6_output(struct net *net, struct sock *sk, struct sk_buff *skb)
struct net_device *dev = skb_dst(skb)->dev;
struct inet6_dev *idev = ip6_dst_idev(skb_dst(skb));
+ skb->protocol = htons(ETH_P_IPV6);
+ skb->dev = dev;
+
if (unlikely(idev->cnf.disable_ipv6)) {
IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
kfree_skb(skb);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: Fix inconsistent teardown and release of private netdev state.
From: David Miller @ 2017-06-09 18:54 UTC (permalink / raw)
To: stephen; +Cc: netdev
In-Reply-To: <20170609102104.34c26568@xeon-e3>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 9 Jun 2017 10:21:04 -0700
> Is there anything in Documentation/networking/netdevices.txt about this to
> avoid any future issues?
You asked me about this last time, and I did not forget about it.
I sincerely lack the time to do a writeup about it, and I felt that
delaying the fix for another week or two until I find that magical
non-existing timeframe to write the docs was not beneficial for
users at all.
^ permalink raw reply
* Re: [PATCH net] Fix an intermittent pr_emerg warning about lo becoming free.
From: Krister Johansen @ 2017-06-09 18:43 UTC (permalink / raw)
To: Cong Wang
Cc: Krister Johansen, David Miller, Eric Dumazet,
Linux Kernel Network Developers, Kaiwen Xu
In-Reply-To: <CAM_iQpUaenZa0o-eyt91sAE6SF+qk6Thh5vpKqPLXSc6xpjPqg@mail.gmail.com>
On Fri, Jun 09, 2017 at 11:18:44AM -0700, Cong Wang wrote:
> On Thu, Jun 8, 2017 at 1:12 PM, Krister Johansen
> <kjlx@templeofstupid.com> wrote:
> > The way this works is that if there's still a reference on the dst entry
> > at the time we try to free it, it gets placed in the gc list by
> > __dst_free and the dst_destroy() call is invoked by the gc task once the
> > refcount is 0. If the gc task processes a 10th or less of its entries
> > on a single pass, it inreases the amount of time it waits between gc
> > intervals.
> >
> > Looking at the gc_task intervals, they started at 663ms when we invoked
> > __dst_free(). After that, they increased to 1663, 3136, 5567, 8191,
> > 10751, and 14848. The release that set the refcnt to 0 on our dst entry
> > occurred after the gc_task was enqueued for 14 second interval so we had
> > to wait longer than the warning time in wait_allrefs in order for the
> > dst entry to get free'd and the hold on 'lo' to be released.
> >
>
> I am glad to see you don't have a dst leak here.
>
> But from my experience of a similar bug (refcnt wait on lo), this goes
> infinitely rather than just 14sec, so it looked more like a real leak than
> just a gc delay. So in your case, this annoying warning eventually
> disappears, right?
That's correct. The problem occurs intermittently, and the warnings are
less frequent than the interval in netdev_wait_allrefs(). At least when
I observed it, it tended to conincide with our controlplane canary
issuing an API call that lead to a network namespace teardown on the
dataplane.
Sometimes, the message would look like this:
unregister_netdevice: waiting for lo to become free. Usage count = 0
The dst entries were getting released, it's just that often our dst
cache gc interval was longer than the warning interval in wait_allrefs.
The other concern was that because the wait_allrefs happens in the
netdev_todo path, a long gc interval can cause the rtnl_lock hold times
to be much longer than necessary if this bug is encountered.
-K
^ permalink raw reply
* Re: [PATCH net] Fix an intermittent pr_emerg warning about lo becoming free.
From: Cong Wang @ 2017-06-09 18:18 UTC (permalink / raw)
To: Krister Johansen
Cc: David Miller, Eric Dumazet, Linux Kernel Network Developers,
Kaiwen Xu
In-Reply-To: <20170608201238.GC2553@templeofstupid.com>
On Thu, Jun 8, 2017 at 1:12 PM, Krister Johansen
<kjlx@templeofstupid.com> wrote:
> After looking through the list of callbacks that the netdevice notifiers
> invoke in this path, it appears that the dst_dev_event is the most
> interesting. The dst_ifdown path places a hold on the loopback_dev as
> part of releasing the dev associated with the original dst cache entry.
> Most of our notifier callbacks are straight-forward, but this one a)
> looks complex, and b) places a hold on the network interface in
> question.
>
> I constructed a new bcc script that watches various events in the
> liftime of a dst cache entry. Note that dst_ifdown will take a hold on
> the loopback device until the invalidated dst entry gets freed.
>
Yeah, this is what I observed when Kevin (Cc'ed) reported a similar
(if not same) bug, I thought we have a refcnt leak on dst.
...
> The way this works is that if there's still a reference on the dst entry
> at the time we try to free it, it gets placed in the gc list by
> __dst_free and the dst_destroy() call is invoked by the gc task once the
> refcount is 0. If the gc task processes a 10th or less of its entries
> on a single pass, it inreases the amount of time it waits between gc
> intervals.
>
> Looking at the gc_task intervals, they started at 663ms when we invoked
> __dst_free(). After that, they increased to 1663, 3136, 5567, 8191,
> 10751, and 14848. The release that set the refcnt to 0 on our dst entry
> occurred after the gc_task was enqueued for 14 second interval so we had
> to wait longer than the warning time in wait_allrefs in order for the
> dst entry to get free'd and the hold on 'lo' to be released.
>
I am glad to see you don't have a dst leak here.
But from my experience of a similar bug (refcnt wait on lo), this goes
infinitely rather than just 14sec, so it looked more like a real leak than
just a gc delay. So in your case, this annoying warning eventually
disappears, right?
Thanks.
^ permalink raw reply
* Re: [PATCH net] ipv4: igmp: fix a use after free
From: Xin Long @ 2017-06-09 18:05 UTC (permalink / raw)
To: Cong Wang; +Cc: Eric Dumazet, Andrey Konovalov, David S. Miller, netdev
In-Reply-To: <CAM_iQpVcLiE3ru1ukq9CCTvTN_+-77XapOWT=1EgF0DgB224-w@mail.gmail.com>
On Sat, Jun 10, 2017 at 1:01 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Jun 9, 2017 at 8:56 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Fri, 2017-06-09 at 14:24 +0800, Xin Long wrote:
>>> On Fri, Jun 9, 2017 at 8:59 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>
>>> > On Thu, Jun 8, 2017 at 1:33 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> >> I mentioned (in https://lkml.org/lkml/2017/5/31/619 ) that we might need
>>> >> to defer freeing after rcu grace period but for some reason decided it
>>> >> was not needed.
>>> Yes, this one could fix it.
>>>
>>> >
>>> > This one makes sense, it is the second time I saw the use-after-free
>>> > in igmp code, both are because we don't respect the RCU rule to free
>>> > an element in the list.
>>> >
>>> >>
>>> >> What about :
>>> >
>>> > But not sure if all ip_ma_put() callers want ip_mc_clear_src().
>>> If that's problem, there may be another way:
>>>
>>> leave ip_mc_clear_src as it is, just add pmc->lock to protect this call.
>>>
>>> this use-after-free was actually caused by using pmc->sources/tomb
>>> in add_grec while ip_mc_clear_src is freeing them. add_grec is already
>>> under pmc->lock, so to add pmc->lock for ip_mc_clear_src should be
>>> enough to protect the list pmc->sources/tomb.
>>>
>>> wdyt ?
>>
>> This would we weird.
>>
>> When we free skb components, we do not grab a spinlock.
>>
>> When we free something, just make sure we must be the last user of it.
>>
>> RCU rules -> Must respect RCU grace period before delete.
>>
>> No need for extra spinlock.
>
> This is what I thought in my first response, until I realized
> it is not pure RCU, otherwise pmc->lock should not be taken
> in igmpv3_send_cr(). It seems the code is mixing the use
> of spinlock and RCU.
rcu lock is for pmc not being freed, and spinlock is for pmc's
members' modification. is there some rule these two locks
should be mixed?
>
> We need RCU anyway, ip_check_mc_rcu() is the real fast
> path where we don't take spinlock. I think we will need more
> work.
It seems all add_grec() callings needs spinlock, maybe add_grec
modifies pmc's member. it's hard to drop spinlock.
from ip_check_mc_rcu you mentioned about, it should be right
to call ip_mc_clear_src after rcu grace period, like Eric's patch.
^ 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