* Re: [patch net 0/4] mlxsw: Couple of fixes
From: David Miller @ 2016-10-20 15:11 UTC (permalink / raw)
To: jiri; +Cc: netdev, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1476972345-31568-1-git-send-email-jiri@resnulli.us>
All applied, thanks Jiri.
^ permalink raw reply
* Re: [Patch net] net: saving irq context for peernet2id()
From: Stephen Smalley @ 2016-10-20 14:58 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: e, paul
In-Reply-To: <1476946352-15770-1-git-send-email-xiyou.wangcong@gmail.com>
On 10/20/2016 02:52 AM, Cong Wang wrote:
> A kernel warning inside __local_bh_enable_ip() was reported by people
> running SELinux, this is caused due to some SELinux functions
> (indirectly) call peernet2id() with IRQ disabled in process context,
> when we re-enable BH with IRQ disabled kernel complains. Shut up this
> warning by saving IRQ context in peernet2id(), BH is still implicitly
> disabled.
Not sure this suffices; kill_fasync() -> send_sigio() ->
send_sigio_to_task() -> sigio_perm() -> security_file_send_sigiotask()
-> selinux_file_send_sigiotask() -> ... -> audit_log() -> ... ->
peernet2id()
>
> Fixes: bc51dddf98c9 ("netns: avoid disabling irq for netns id")
> Reported-by: Stephen Smalley <sds@tycho.nsa.gov>
> Reported-by: Elad Raz <e@eladraz.com>
> Tested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/core/net_namespace.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 989434f..17b52a2 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -230,11 +230,16 @@ int peernet2id_alloc(struct net *net, struct net *peer)
> /* This function returns, if assigned, the id of a peer netns. */
> int peernet2id(struct net *net, struct net *peer)
> {
> + unsigned long flags;
> int id;
>
> - spin_lock_bh(&net->nsid_lock);
> + /* This is ugly, technically we only need to disable BH, however some
> + * callers (indirectly) call this with IRQ disabled, which implies
> + * that we should save the IRQ context here.
> + */
> + spin_lock_irqsave(&net->nsid_lock, flags);
> id = __peernet2id(net, peer);
> - spin_unlock_bh(&net->nsid_lock);
> + spin_unlock_irqrestore(&net->nsid_lock, flags);
> return id;
> }
> EXPORT_SYMBOL(peernet2id);
>
^ permalink raw reply
* RE: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
From: Jon Maloy @ 2016-10-20 14:51 UTC (permalink / raw)
To: Ben Hutchings, Ying Xue; +Cc: netdev@vger.kernel.org, Qian Zhang, Eric Dumazet
In-Reply-To: <1476967585.2709.52.camel@decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Thursday, 20 October, 2016 08:46
> To: Ying Xue <ying.xue0@gmail.com>; Jon Maloy <jon.maloy@ericsson.com>
> Cc: netdev@vger.kernel.org; Qian Zhang <zhangqian-c@360.cn>; Eric Dumazet
> <edumazet@google.com>
> Subject: Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
>
> On Thu, 2016-10-20 at 17:30 +0800, Ying Xue wrote:
> > On 10/19/2016 10:16 AM, Ben Hutchings wrote:
> > > Qian Zhang (张谦) reported a potential socket buffer overflow in
> > > tipc_msg_build(). The minimum fragment length needs to be checked
> > > against the maximum packet size, which is based on the link MTU.
> [...]
> > >
> > > --- a/net/tipc/msg.c
> > > +++ b/net/tipc/msg.c
> > > @@ -274,6 +274,10 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct
> msghdr *m,
> > > > > goto error;
> > > > > }
> > >
> > > > > + /* Check that fragment and message header will fit */
> > > > > + if (INT_H_SIZE + mhsz > pktmax)
> > > + return -EMSGSIZE;
> >
> >
> > The "mhsz" represents the size of tipc packet header for current socket,
> > INT_H_SIZE indicates the size of tipc internal message header. So it
> > seems unreasonable to identify whether the sum of both header sizes is
> > bigger than MTU size. In my opinion, it's better to use MAX_H_SIZE to
> > compare it with pktmax. If MAX_H_SIZE is bigger than pktmax, we should
> > return EMSGSIZE error code.
>
> At this point we're about to copy INT_H_SIZE + mhsz bytes into the
> first fragment. If that's already limited to be less than or equal to
> MAX_H_SIZE, comparing with MAX_H_SIZE would be fine. But if MAX_H_SIZE
> is the maximum value of mhsz, that won't be good enough.
MAX_H_SIZE is 60 bytes, but in practice you will never see an mhsz larger than the biggest header we are actually using, which is MCAST_H_SIZE (==44 bytes).
INT_H_SIZE is 40 bytes, so you are in reality testing for whether we have an mtu < 84 bytes.
You won't find any interfaces or protocols that come even close to this limitation, so to me this test is redundant.
Regards
///jon
>
> Ben.
>
> > > +
> > > > > /* Prepare reusable fragment header */
> > > > > tipc_msg_init(msg_prevnode(mhdr), &pkthdr,
> MSG_FRAGMENTER,
> > > > > FIRST_FRAGMENT, INT_H_SIZE,
> msg_destnode(mhdr));
> > >
> >
> >
> >
> --
> Ben Hutchings
> Never put off till tomorrow what you can avoid all together.
^ permalink raw reply
* Re: [PATCH 1/4] kconfig: introduce the "imply" keyword
From: Edward Cree @ 2016-10-20 14:52 UTC (permalink / raw)
To: Nicolas Pitre, John Stultz, Richard Cochran, Yann E MORIN
Cc: Thomas Gleixner, Josh Triplett, netdev, linux-kbuild,
linux-kernel
In-Reply-To: <1476920573-14384-2-git-send-email-nicolas.pitre@linaro.org>
On 20/10/16 00:42, Nicolas Pitre wrote:
> diff --git a/Documentation/kbuild/kconfig-language.txt b/Documentation/kbuild/kconfig-language.txt
> index 069fcb3eef..c96127f648 100644
> --- a/Documentation/kbuild/kconfig-language.txt
> +++ b/Documentation/kbuild/kconfig-language.txt
> @@ -113,6 +113,33 @@ applicable everywhere (see syntax).
> That will limit the usefulness but on the other hand avoid
> the illegal configurations all over.
>
> +- weak reverse dependencies: "imply" <symbol> ["if" <expr>]
> + This is similar to "select" as it enforces a lower limit on another
> + symbol except that the "implied" config symbol's value may still be
> + set to n from a direct dependency or with a visible prompt.
> + Given the following example:
> +
> + config FOO
> + tristate
> + imply BAZ
> +
> + config BAZ
> + tristate
> + depends on BAr
> +
> + The following values are possible:
> +
> + FOO BAR BAR's default choice for BAZ
Should the third column not be "BAZ's default"?
> + --------------- --------------- --------------- --------------
> + n y n N/m/y
> + m y m M/y/n
> + y y y Y/n
> + y n * N
Also, I don't think having any FOO=y should preclude BAZ=m. Suppose both
FOO and FOO2 imply BAZ, FOO=y and FOO2=m. Then if BAZ-features are only
desired for driver FOO2, BAz=m makes sense.
There is also the case of drivers with the ability to detect at runtime
whether BAZ is present, rather than making the decision at build time, but
I'm not sure how common that is.
-Ed
^ permalink raw reply
* Re: [PATCH v4 net-next 1/7] ipv6: Fix Makefile conditional to use CONFIG_INET
From: David Miller @ 2016-10-20 14:38 UTC (permalink / raw)
To: tom; +Cc: netdev, kernel-team
In-Reply-To: <CALx6S34ypSKUg715g1K783ze95ko1SeSA+Yb0JFqvfsSHwxQug@mail.gmail.com>
From: Tom Herbert <tom@herbertland.com>
Date: Wed, 19 Oct 2016 20:16:01 -0700
> Please don't apply this. It's like whack-a-mole trying to resolve all
> the dependencies. I'll resubmit the series without this.
Ok.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: ethtool -x full support
From: Eric Dumazet @ 2016-10-20 14:26 UTC (permalink / raw)
To: Mintz, Yuval; +Cc: David Miller, netdev, Ariel Elior
In-Reply-To: <BL2PR07MB2306E14F54D13CA215D8923C8DD50@BL2PR07MB2306.namprd07.prod.outlook.com>
On Thu, 2016-10-20 at 12:27 +0000, Mintz, Yuval wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > Implement ethtool -x full support, so that rss key can be fetched
> > > instead of assuming it matches /proc/sys/net/core/netdev_rss_key
> > > content.
> >
> > I'll send a V2, tested with CONFIG_BNX2X_SRIOV=y ;)
> >
>
> Hi Eric,
>
> Thanks for this. Are you planning on sending v2 or do you want us to take over?
>
> BTW, what's the benefit of changing the RSS keys? Giving the hash properties,
> in which scenarios would we gain by switching keys?
I will send a v2
Changing keys once a day or so might be important for security
purposes.
I wont give details here ;)
Thanks.
^ permalink raw reply
* Re: [PATCH 3/4] ptp_clock: allow for it to be optional
From: Richard Cochran @ 2016-10-20 14:06 UTC (permalink / raw)
To: Nicolas Pitre
Cc: John Stultz, Yann E MORIN, Thomas Gleixner, Josh Triplett, netdev,
linux-kbuild, linux-kernel
In-Reply-To: <1476920573-14384-4-git-send-email-nicolas.pitre@linaro.org>
On Wed, Oct 19, 2016 at 07:42:52PM -0400, Nicolas Pitre wrote:
> +static inline void ptp_clock_event(struct ptp_clock *ptp,
> + struct ptp_clock_event *event)
> +{ (void)event; }
Just out of curiosity, why do you need that cast?
(I thought the kernel's gcc settings don't enable the warning for
unused arguments.)
Thanks,
Richard
^ permalink raw reply
* [patch net 4/4] mlxsw: pci: Fix reset wait for SwitchX2
From: Jiri Pirko @ 2016-10-20 14:05 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1476972345-31568-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
SwitchX2 firmware does not implement reset done yet. Moreover, when
busy-polled for ready magic, that slows down firmware and reset takes
longer than the defined timeout, causing initialization to fail.
So restore the previous behaviour and just sleep in this case.
Fixes: 233fa44bd67a ("mlxsw: pci: Implement reset done check")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index e742bd4..912f71f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1838,11 +1838,17 @@ static const struct mlxsw_bus mlxsw_pci_bus = {
.cmd_exec = mlxsw_pci_cmd_exec,
};
-static int mlxsw_pci_sw_reset(struct mlxsw_pci *mlxsw_pci)
+static int mlxsw_pci_sw_reset(struct mlxsw_pci *mlxsw_pci,
+ const struct pci_device_id *id)
{
unsigned long end;
mlxsw_pci_write32(mlxsw_pci, SW_RESET, MLXSW_PCI_SW_RESET_RST_BIT);
+ if (id->device == PCI_DEVICE_ID_MELLANOX_SWITCHX2) {
+ msleep(MLXSW_PCI_SW_RESET_TIMEOUT_MSECS);
+ return 0;
+ }
+
wmb(); /* reset needs to be written before we read control register */
end = jiffies + msecs_to_jiffies(MLXSW_PCI_SW_RESET_TIMEOUT_MSECS);
do {
@@ -1909,7 +1915,7 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
mlxsw_pci->pdev = pdev;
pci_set_drvdata(pdev, mlxsw_pci);
- err = mlxsw_pci_sw_reset(mlxsw_pci);
+ err = mlxsw_pci_sw_reset(mlxsw_pci, id);
if (err) {
dev_err(&pdev->dev, "Software reset failed\n");
goto err_sw_reset;
--
2.5.5
^ permalink raw reply related
* [patch net 3/4] mlxsw: switchx2: Fix ethernet port initialization
From: Jiri Pirko @ 2016-10-20 14:05 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1476972345-31568-1-git-send-email-jiri@resnulli.us>
From: Elad Raz <eladr@mellanox.com>
When creating an ethernet port fails, we must move the port to disable,
otherwise putting the port in switch partition 0 (ETH) or 1 (IB) will
always fails.
Fixes: 31557f0f9755 ("mlxsw: Introduce Mellanox SwitchX-2 ASIC support")
Signed-off-by: Elad Raz <eladr@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
index c0c23e2..92bda87 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/switchx2.c
@@ -1088,6 +1088,7 @@ static int mlxsw_sx_port_create(struct mlxsw_sx *mlxsw_sx, u8 local_port)
err_port_admin_status_set:
err_port_mtu_set:
err_port_speed_set:
+ mlxsw_sx_port_swid_set(mlxsw_sx_port, MLXSW_PORT_SWID_DISABLED_PORT);
err_port_swid_set:
err_port_system_port_mapping_set:
port_not_usable:
--
2.5.5
^ permalink raw reply related
* [patch net 2/4] mlxsw: spectrum_router: Make mlxsw_sp_router_fib4_del return void and remove warn
From: Jiri Pirko @ 2016-10-20 14:05 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1476972345-31568-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
The function return value is not checked anywhere. Also, the warning
causes huge slowdown when removing large number of FIB entries which
were not offloaded, because of ordering issue. Ido's preparing
a patchset to fix the ordering issue, but that is definitelly not
net tree material.
Fixes: b45f64d16d45 ("mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 1b3a2cb..f3d50d3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1820,19 +1820,17 @@ static int mlxsw_sp_router_fib4_add(struct mlxsw_sp *mlxsw_sp,
return err;
}
-static int mlxsw_sp_router_fib4_del(struct mlxsw_sp *mlxsw_sp,
- struct fib_entry_notifier_info *fen_info)
+static void mlxsw_sp_router_fib4_del(struct mlxsw_sp *mlxsw_sp,
+ struct fib_entry_notifier_info *fen_info)
{
struct mlxsw_sp_fib_entry *fib_entry;
if (mlxsw_sp->router.aborted)
- return 0;
+ return;
fib_entry = mlxsw_sp_fib_entry_find(mlxsw_sp, fen_info);
- if (!fib_entry) {
- dev_warn(mlxsw_sp->bus_info->dev, "Failed to find FIB4 entry being removed.\n");
- return -ENOENT;
- }
+ if (!fib_entry)
+ return;
if (fib_entry->ref_count == 1) {
mlxsw_sp_fib_entry_del(mlxsw_sp, fib_entry);
@@ -1840,7 +1838,6 @@ static int mlxsw_sp_router_fib4_del(struct mlxsw_sp *mlxsw_sp,
}
mlxsw_sp_fib_entry_put(mlxsw_sp, fib_entry);
- return 0;
}
static int mlxsw_sp_router_set_abort_trap(struct mlxsw_sp *mlxsw_sp)
--
2.5.5
^ permalink raw reply related
* [patch net 1/4] mlxsw: spectrum_router: Use correct tree index for binding
From: Jiri Pirko @ 2016-10-20 14:05 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1476972345-31568-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
By a mistake, there is tree index 0 passed to RALTB. Should be
MLXSW_SP_LPM_TREE_MIN.
Fixes: b45f64d16d45 ("mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls")
Reported-by: Yotam Gigi <yotamg@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 78fc557d..1b3a2cb 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1862,7 +1862,8 @@ static int mlxsw_sp_router_set_abort_trap(struct mlxsw_sp *mlxsw_sp)
if (err)
return err;
- mlxsw_reg_raltb_pack(raltb_pl, 0, MLXSW_REG_RALXX_PROTOCOL_IPV4, 0);
+ mlxsw_reg_raltb_pack(raltb_pl, 0, MLXSW_REG_RALXX_PROTOCOL_IPV4,
+ MLXSW_SP_LPM_TREE_MIN);
err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(raltb), raltb_pl);
if (err)
return err;
--
2.5.5
^ permalink raw reply related
* [patch net 0/4] mlxsw: Couple of fixes
From: Jiri Pirko @ 2016-10-20 14:05 UTC (permalink / raw)
To: netdev; +Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz
From: Jiri Pirko <jiri@mellanox.com>
Elad Raz (1):
mlxsw: switchx2: Fix ethernet port initialization
Jiri Pirko (3):
mlxsw: spectrum_router: Use correct tree index for binding
mlxsw: spectrum_router: Make mlxsw_sp_router_fib4_del return void and
remove warn
mlxsw: pci: Fix reset wait for SwitchX2
drivers/net/ethernet/mellanox/mlxsw/pci.c | 10 ++++++++--
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 16 +++++++---------
drivers/net/ethernet/mellanox/mlxsw/switchx2.c | 1 +
3 files changed, 16 insertions(+), 11 deletions(-)
--
2.5.5
^ permalink raw reply
* Re: [PATCH 4/4] posix-timers: make it configurable
From: Richard Cochran @ 2016-10-20 14:02 UTC (permalink / raw)
To: Nicolas Pitre
Cc: John Stultz, Yann E MORIN, Thomas Gleixner, Josh Triplett, netdev,
linux-kbuild, linux-kernel
In-Reply-To: <1476920573-14384-5-git-send-email-nicolas.pitre@linaro.org>
On Wed, Oct 19, 2016 at 07:42:53PM -0400, Nicolas Pitre wrote:
> +config POSIX_TIMERS
> + bool "Posix Clocks & timers" if EXPERT
> + default y
> + help
> + This includes native support for POSIX timers to the kernel.
> + Most embedded systems may have no use for them and therefore they
This sentence doesn't make sense. How about:
Some embedded systems have no use for them ...
> + can be configured out to reduce the size of the kernel image.
Thanks,
Richard
^ permalink raw reply
* [PATCH net v3] net: add recursion limit to GRO
From: Sabrina Dubroca @ 2016-10-20 13:58 UTC (permalink / raw)
To: netdev
Cc: Sabrina Dubroca, Eric Dumazet, Tom Herbert, Jiri Benc,
Hannes Frederic Sowa
Currently, GRO can do unlimited recursion through the gro_receive
handlers. This was fixed for tunneling protocols by limiting tunnel GRO
to one level with encap_mark, but both VLAN and TEB still have this
problem. Thus, the kernel is vulnerable to a stack overflow, if we
receive a packet composed entirely of VLAN headers.
This patch adds a recursion counter to the GRO layer to prevent stack
overflow. When a gro_receive function hits the recursion limit, GRO is
aborted for this skb and it is processed normally. This recursion
counter is put in the GRO CB, but could be turned into a percpu counter
if we run out of space in the CB.
Thanks to Vladimír Beneš <vbenes@redhat.com> for the initial bug report.
Fixes: CVE-2016-7039
Fixes: 9b174d88c257 ("net: Add Transparent Ethernet Bridging GRO support.")
Fixes: 66e5133f19e9 ("vlan: Add GRO support for non hardware accelerated vlan")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
v2: add unlikely() annotations, suggested by Eric Dumazet
benchmark didn't show a difference between CB and pcpu variable for
the recursion counter, so I'm sticking with the CB
v3: add call_gro_receive_sk, suggested by Tom Herbert
drivers/net/geneve.c | 2 +-
drivers/net/vxlan.c | 2 +-
include/linux/netdevice.h | 39 ++++++++++++++++++++++++++++++++++++++-
net/8021q/vlan.c | 2 +-
net/core/dev.c | 1 +
net/ethernet/eth.c | 2 +-
net/ipv4/af_inet.c | 2 +-
net/ipv4/fou.c | 4 ++--
net/ipv4/gre_offload.c | 2 +-
net/ipv4/udp_offload.c | 2 +-
net/ipv6/ip6_offload.c | 2 +-
11 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 3c20e87bb761..16af1ce99233 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -453,7 +453,7 @@ static struct sk_buff **geneve_gro_receive(struct sock *sk,
skb_gro_pull(skb, gh_len);
skb_gro_postpull_rcsum(skb, gh, gh_len);
- pp = ptype->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ptype->callbacks.gro_receive, head, skb);
flush = 0;
out_unlock:
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index e7d16687538b..c1639a3e95a4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -583,7 +583,7 @@ static struct sk_buff **vxlan_gro_receive(struct sock *sk,
}
}
- pp = eth_gro_receive(head, skb);
+ pp = call_gro_receive(eth_gro_receive, head, skb);
flush = 0;
out:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 465e128699a1..91ee3643ccc8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2169,7 +2169,10 @@ struct napi_gro_cb {
/* Used to determine if flush_id can be ignored */
u8 is_atomic:1;
- /* 5 bit hole */
+ /* Number of gro_receive callbacks this packet already went through */
+ u8 recursion_counter:4;
+
+ /* 1 bit hole */
/* used to support CHECKSUM_COMPLETE for tunneling protocols */
__wsum csum;
@@ -2180,6 +2183,40 @@ struct napi_gro_cb {
#define NAPI_GRO_CB(skb) ((struct napi_gro_cb *)(skb)->cb)
+#define GRO_RECURSION_LIMIT 15
+static inline int gro_recursion_inc_test(struct sk_buff *skb)
+{
+ return ++NAPI_GRO_CB(skb)->recursion_counter == GRO_RECURSION_LIMIT;
+}
+
+typedef struct sk_buff **(*gro_receive_t)(struct sk_buff **, struct sk_buff *);
+static inline struct sk_buff **call_gro_receive(gro_receive_t cb,
+ struct sk_buff **head,
+ struct sk_buff *skb)
+{
+ if (unlikely(gro_recursion_inc_test(skb))) {
+ NAPI_GRO_CB(skb)->flush |= 1;
+ return NULL;
+ }
+
+ return cb(head, skb);
+}
+
+typedef struct sk_buff **(*gro_receive_sk_t)(struct sock *, struct sk_buff **,
+ struct sk_buff *);
+static inline struct sk_buff **call_gro_receive_sk(gro_receive_sk_t cb,
+ struct sock *sk,
+ struct sk_buff **head,
+ struct sk_buff *skb)
+{
+ if (unlikely(gro_recursion_inc_test(skb))) {
+ NAPI_GRO_CB(skb)->flush |= 1;
+ return NULL;
+ }
+
+ return cb(sk, head, skb);
+}
+
struct packet_type {
__be16 type; /* This is really htons(ether_type). */
struct net_device *dev; /* NULL is wildcarded here */
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 8de138d3306b..f2531ad66b68 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -664,7 +664,7 @@ static struct sk_buff **vlan_gro_receive(struct sk_buff **head,
skb_gro_pull(skb, sizeof(*vhdr));
skb_gro_postpull_rcsum(skb, vhdr, sizeof(*vhdr));
- pp = ptype->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ptype->callbacks.gro_receive, head, skb);
out_unlock:
rcu_read_unlock();
diff --git a/net/core/dev.c b/net/core/dev.c
index b09ac57f4348..dbc871306910 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4511,6 +4511,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
NAPI_GRO_CB(skb)->flush = 0;
NAPI_GRO_CB(skb)->free = 0;
NAPI_GRO_CB(skb)->encap_mark = 0;
+ NAPI_GRO_CB(skb)->recursion_counter = 0;
NAPI_GRO_CB(skb)->is_fou = 0;
NAPI_GRO_CB(skb)->is_atomic = 1;
NAPI_GRO_CB(skb)->gro_remcsum_start = 0;
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 66dff5e3d772..02acfff36028 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -439,7 +439,7 @@ struct sk_buff **eth_gro_receive(struct sk_buff **head,
skb_gro_pull(skb, sizeof(*eh));
skb_gro_postpull_rcsum(skb, eh, sizeof(*eh));
- pp = ptype->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ptype->callbacks.gro_receive, head, skb);
out_unlock:
rcu_read_unlock();
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1effc986739e..9648c97e541f 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1391,7 +1391,7 @@ struct sk_buff **inet_gro_receive(struct sk_buff **head, struct sk_buff *skb)
skb_gro_pull(skb, sizeof(*iph));
skb_set_transport_header(skb, skb_gro_offset(skb));
- pp = ops->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
out_unlock:
rcu_read_unlock();
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index cf50f7e2b012..030d1531e897 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -249,7 +249,7 @@ static struct sk_buff **fou_gro_receive(struct sock *sk,
if (!ops || !ops->callbacks.gro_receive)
goto out_unlock;
- pp = ops->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
out_unlock:
rcu_read_unlock();
@@ -441,7 +441,7 @@ static struct sk_buff **gue_gro_receive(struct sock *sk,
if (WARN_ON_ONCE(!ops || !ops->callbacks.gro_receive))
goto out_unlock;
- pp = ops->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
flush = 0;
out_unlock:
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 96e0efecefa6..d5cac99170b1 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -229,7 +229,7 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
/* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
skb_gro_postpull_rcsum(skb, greh, grehlen);
- pp = ptype->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ptype->callbacks.gro_receive, head, skb);
flush = 0;
out_unlock:
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index f9333c963607..b2be1d9757ef 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -295,7 +295,7 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
- pp = udp_sk(sk)->gro_receive(sk, head, skb);
+ pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
out_unlock:
rcu_read_unlock();
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index e7bfd55899a3..1fcf61f1cbc3 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -246,7 +246,7 @@ static struct sk_buff **ipv6_gro_receive(struct sk_buff **head,
skb_gro_postpull_rcsum(skb, iph, nlen);
- pp = ops->callbacks.gro_receive(head, skb);
+ pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
out_unlock:
rcu_read_unlock();
--
2.10.0
^ permalink raw reply related
* Re: [Patch net v2] ipv6: fix a potential deadlock in do_ipv6_setsockopt()
From: Eric Dumazet @ 2016-10-20 13:47 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, sploving1, Marcelo Ricardo Leitner
In-Reply-To: <1476945312-15572-1-git-send-email-xiyou.wangcong@gmail.com>
On Wed, 2016-10-19 at 23:35 -0700, Cong Wang wrote:
> Baozeng reported this deadlock case:
...
> +
> +void ipv6_sock_mc_close(struct sock *sk)
> +{
> + struct ipv6_pinfo *np = inet6_sk(sk);
> +
> + if (!rcu_access_pointer(np->ipv6_mc_list))
> + return;
I wonder if rcu_dereference_protected(..., lockdep_sock_is_held(sk))
could be used instead, to get lockdep support ?
> + rtnl_lock();
> + __ipv6_sock_mc_close(sk);
> rtnl_unlock();
> }
>
Thanks !
^ permalink raw reply
* Re: [PATCH 1/9] ipv6: implement dataplane support for rthdr type 4 (Segment Routing Header)
From: David Lebrun @ 2016-10-20 13:04 UTC (permalink / raw)
To: Tom Herbert; +Cc: Linux Kernel Network Developers
In-Reply-To: <CALx6S35sU+aw3UvMTknj8SjJP4O=Q-9eVV3OC640Qzces_eE7w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
On 10/17/2016 07:01 PM, Tom Herbert wrote:
>> > +
>> > + if (skb->ip_summed == CHECKSUM_COMPLETE)
>> > + skb->ip_summed = CHECKSUM_NONE;
>> > +
> Because the packet is being changed? Would it make sense to update the
> checksum complete value based on the changes being made. Consider the
> case that the next hop is local to the host (someone may try to
> implement network virtualization this way).
>
Rethinking about that: even if the next hop is local, I am not sure to
see the benefits of updating the checksum instead of setting
CHECKSUM_NONE. For example, if the next and final hop is local and the
packet carries a TCP payload, tcp_checksum_complete() would force the
recomputation of the checksum anyway (unless ip_summed ==
CHECKSUM_UNNECESSARY).
So I fail to see a path where updating the checksum would be beneficial.
Am I missing something ?
David
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 163 bytes --]
^ permalink raw reply
* RE: [RFC 2/6] qed: Add iSCSI out of order packet handling.
From: Mintz, Yuval @ 2016-10-20 12:58 UTC (permalink / raw)
To: Hannes Reinecke, Rangankar, Manish, lduncan@suse.com,
cleech@redhat.com
Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com,
linux-scsi@vger.kernel.org, netdev@vger.kernel.org,
Dept-Eng QLogic Storage Upstream, Easi, Arun
In-Reply-To: <f8c5d8d1-0e69-00c2-9349-de611c4b6c08@suse.de>
> > This patch adds out of order packet handling for hardware offloaded
> > iSCSI. Out of order packet handling requires driver buffer allocation
> > and assistance.
> >
> > Signed-off-by: Arun Easi <arun.easi@cavium.com>
> > Signed-off-by: Yuval Mintz <yuval.mintz@cavium.com>
> >
> Hmm. The entire out-of-order handling is pretty generic. I really wonder
> if this doesn't apply to iSCSI in general; surely iscsi_tcp suffers from
> the same problem, no?
> If so, wouldn't it be better to move it into generic (iSCSI) code so
> that all implementations would benefit from it?
[disclaimer - I'm far from knowledgeable in iscsi ]
I agree that the problem of out-of-order handling is probably generic,
but our solution is very device oriented.
As the device lacks [a lot of] internal memory, it uses the host memory
for out-of-order buffers and the driver assistance in pushing them when
they are needed.
>From driver perspective, all the data is completely opaque; All it does is
follow the firmware's guidance in storing & re-transmitting buffers when
required.
Now, I guess the logic could be divided between hardware-specifics -
Interaction with 'client' [in our case, device's firmware], to receive
new data, instructions regarding placement and re-transmission,
and a lower generic data structure which supports manipulation of
buffers [push-left, push-right, join, etc.].
But given that the data-structure would completely lacks all
protocol-knowledge [as our implementation doesn't have nor require such],
I think there would be very little gain - we might find out that as much
as 80% of the code is device interaction, and the remaining so-called
'generic' data-structure won't be that useful to other clients as it was
closely tied to our device needs and API.
Either way, placing this under iscsi would probably be insufficient for our
future needs, as our qed-iwarp driver would also require this functionality.
Thanks,
Yuval
^ permalink raw reply
* Re: Need help with mdiobus_register and phy
From: Zefir Kurtisi @ 2016-10-20 12:55 UTC (permalink / raw)
To: Timur Tabi, Andrew Lunn; +Cc: Florian Fainelli, netdev
In-Reply-To: <58076403.3060608@codeaurora.org>
On 10/19/2016 02:16 PM, Timur Tabi wrote:
> Zefir Kurtisi wrote:
>> Ok then, if you can wait some days, I'll prepare and provide you a more detailed
>> failure report to allow you testing if the issue happens with other NICs.
>
> That sounds great.
>
I am now able again to reproduce the issue reliably, let's see if it is helpful
enough to check if this a problem unique to our setup.
As described earlier, the HW is as follows:
(copper)--[at8031]--(SGMII)--[eTSEC/mpc85xx]
We are using OpenWRT/LEDE as SW platform, therefore I can't do the tests on the
latest kernel but provide a reference to inspect the problem. The kernel used is
4.4.21 with the relevant drivers (ath803x and gianfar_driver) being either clean
or patched with OWRT/LEDE modifications (issue happens in either case).
The process for detecting the failure is rather trivial: continuously put the
Ethernet interface down and up and try to get some data through until it fails.
The script I used to run on the device (over serial) is:
#----------------------------------------------------------
HOST=192.168.1.100
TIMEOUT=20
while true; do
ifconfig eth0 down
ifconfig eth0 up
ping $HOST -c 1 -w $TIMEOUT || {
echo "Ping failed after $TIMEOUT seconds"
exit 1
}
done
#----------------------------------------------------------
In my setup, it takes less than 5 minutes for the script to exit. Once running
into this state, the relevant registers read from at803x are as follows:
* copper status (0x01): 0x796d
* fiber status (0x01): 0x6149
* copper phy specific status (0x11): 0xbc10
* fiber phy specific status (0x11): 0x8100
* copper/fiber status (0x1b): 0x020e
In essence
* the copper link is up with autonegotiation completed
(switch/peer link indication is also up), but
* the SGMII link is down
(bits LINK and MR_AN_COMPLETE in PHY specific status fiber cleared)
* although SGMII is synchronized (SYNC_STATUS bit in same register set)
The SGMII link does not recover from this state thereafter, the only reliable way
to get it up again I found is to do a power down/up cycle at the fiber/SGMII side,
which is what the patch I contributed does.
Now the interesting (and new for me) part is: if I remove the at803x driver from
the system and use the generic phy instead, the problem does not happen (or at
least not while running for one full day). I don't see any significant differences
between what at803x does above genphy (gpio based HW reset is not run for 8031 in
at803x_link_change_notify()). Using the genphy is not an option for us, since we
have the PHY also attached to fiber and need dedicated handling.
Hope this helps for the beginning, let me know if you need more information.
Cheers,
Zefir
^ permalink raw reply
* Re: [PATCH net] tipc: Guard against tiny MTU in tipc_msg_build()
From: Ben Hutchings @ 2016-10-20 12:46 UTC (permalink / raw)
To: Ying Xue, Jon Maloy; +Cc: netdev, Qian Zhang, Eric Dumazet
In-Reply-To: <6a69780d-0797-b5af-fc90-04960bbb21c6@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]
On Thu, 2016-10-20 at 17:30 +0800, Ying Xue wrote:
> On 10/19/2016 10:16 AM, Ben Hutchings wrote:
> > Qian Zhang (张谦) reported a potential socket buffer overflow in
> > tipc_msg_build(). The minimum fragment length needs to be checked
> > against the maximum packet size, which is based on the link MTU.
[...]
> >
> > --- a/net/tipc/msg.c
> > +++ b/net/tipc/msg.c
> > @@ -274,6 +274,10 @@ int tipc_msg_build(struct tipc_msg *mhdr, struct msghdr *m,
> > > > goto error;
> > > > }
> >
> > > > + /* Check that fragment and message header will fit */
> > > > + if (INT_H_SIZE + mhsz > pktmax)
> > + return -EMSGSIZE;
>
>
> The "mhsz" represents the size of tipc packet header for current socket,
> INT_H_SIZE indicates the size of tipc internal message header. So it
> seems unreasonable to identify whether the sum of both header sizes is
> bigger than MTU size. In my opinion, it's better to use MAX_H_SIZE to
> compare it with pktmax. If MAX_H_SIZE is bigger than pktmax, we should
> return EMSGSIZE error code.
At this point we're about to copy INT_H_SIZE + mhsz bytes into the
first fragment. If that's already limited to be less than or equal to
MAX_H_SIZE, comparing with MAX_H_SIZE would be fine. But if MAX_H_SIZE
is the maximum value of mhsz, that won't be good enough.
Ben.
> > +
> > > > /* Prepare reusable fragment header */
> > > > tipc_msg_init(msg_prevnode(mhdr), &pkthdr, MSG_FRAGMENTER,
> > > > FIRST_FRAGMENT, INT_H_SIZE, msg_destnode(mhdr));
> >
>
>
>
--
Ben Hutchings
Never put off till tomorrow what you can avoid all together.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH] davinci_emac: fix setting the mac from DT
From: Tony Lindgren @ 2016-10-20 12:41 UTC (permalink / raw)
To: Jeroen Hofstee
Cc: netdev, David S . Miller, Anant Gole, Chaithrika U S, linux-omap
In-Reply-To: <1476904973-13226-1-git-send-email-jhofstee@victronenergy.com>
* Jeroen Hofstee <jhofstee@victronenergy.com> [161019 12:39]:
> commit 9120bd6e9f77 ("net: davinci_emac: Get device dm816x MAC address
> using the cpsw code") sets the mac address to the one stored in the chip
> unconditionally, overwritten the one already set from the device tree.
> This patch makes sure the mac from DT is preserved.
>
> On a am3517 this address is incorrectly read as all zeros, making it
> impossible to set a valid mac address without this patch.
OK, at least I don't have better ideas for fixing this:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* RE: [PATCH net-next] bnx2x: ethtool -x full support
From: Mintz, Yuval @ 2016-10-20 12:27 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev, Ariel Elior
In-Reply-To: <1476818655.5650.69.camel@edumazet-glaptop3.roam.corp.google.com>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Implement ethtool -x full support, so that rss key can be fetched
> > instead of assuming it matches /proc/sys/net/core/netdev_rss_key
> > content.
>
> I'll send a V2, tested with CONFIG_BNX2X_SRIOV=y ;)
>
Hi Eric,
Thanks for this. Are you planning on sending v2 or do you want us to take over?
BTW, what's the benefit of changing the RSS keys? Giving the hash properties,
in which scenarios would we gain by switching keys?
Thanks,
Yuval
^ permalink raw reply
* Re: mlx4: panic during shutdown
From: Tariq Toukan @ 2016-10-20 9:07 UTC (permalink / raw)
To: Sebastian Ott, Yishai Hadas; +Cc: netdev, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1610191617400.1730@schleppi>
Hi Sebastian,
Thanks for the report.
We've encountered this as well, and trying to find the correct way of
solving it.
On 19/10/2016 5:35 PM, Sebastian Ott wrote:
> Hi,
>
> After a userspace update (fedora 23->24) I reproducibly run into the
> following oops during shutdown (on s390):
>
> [ 71.054832] Unable to handle kernel pointer dereference in virtual kernel address space
> [ 71.054835] Failing address: 6b6b6b6b6b6b6000 TEID: 6b6b6b6b6b6b6803
> [ 71.054838] Fault in home space mode while using kernel ASCE.
> [ 71.054847] AS:0000000000f70007 R3:0000000000000024
> [ 71.054883] Oops: 0038 ilc:3 [#1] PREEMPT SMP
> [ 71.054887] Modules linked in: mlx4_ib ib_core mlx4_en ptp pps_core mlx4_core [...]
> [ 71.054912] CPU: 8 PID: 809 Comm: kworker/8:6 Not tainted 4.8.0-02896-g7137af2-dirty #6
> [ 71.054913] Hardware name: IBM 2964 N96 704 (LPAR)
> [ 71.054919] Workqueue: events linkwatch_event
> [ 71.054921] task: 00000000dbea0008 task.stack: 00000000dbea4000
> [ 71.054923] Krnl PSW : 0704e00180000000 000003ff8007a496 (mlx4_en_get_phys_port_id+0x66/0xb0 [mlx4_en])
> [ 71.054933] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> Krnl GPRS: 0000000000000080 0000000000000268 000000000000004e 00000000001c33e0
> [ 71.054937] 000003ff8007a486 0000000000882790 6b6b6b6b6b6b6b6b 0000000000000010
> [ 71.054939] 00000000dbea7b18 6b6b6b6b6b6b6b6b 00000000dbea7b18 00000000e72e0000
> [ 71.054941] 00000000f15ec900 0000000000000000 000003ff8007a486 00000000dbea79c8
> [ 71.054950] Krnl Code: 000003ff8007a486: e310b81c0d14 lgf %r1,55324(%r11)
> 000003ff8007a48c: a71b004b aghi %r1,75
> #000003ff8007a490: eb110003000d sllg %r1,%r1,3
> >000003ff8007a496: e31190000002 ltg %r1,0(%r1,%r9)
> 000003ff8007a49c: a7840015 brc 8,3ff8007a4c6
> 000003ff8007a4a0: 9208a020 mvi 32(%r10),8
> 000003ff8007a4a4: 4130a007 la %r3,7(%r10)
> 000003ff8007a4a8: a7290008 lghi %r2,8
> [ 71.054965] Call Trace:
> [ 71.054969] ([<000003ff8007a486>] mlx4_en_get_phys_port_id+0x56/0xb0 [mlx4_en])
> [ 71.054971] ([<0000000000760b94>] rtnl_fill_ifinfo+0x4ec/0xc90)
> [ 71.054974] ([<0000000000764fae>] rtmsg_ifinfo_build_skb+0x96/0xe8)
> [ 71.054976] ([<0000000000765038>] rtmsg_ifinfo+0x38/0x78)
> [ 71.054978] ([<000000000074150e>] netdev_state_change+0x5e/0x70)
> [ 71.054981] ([<0000000000765ca6>] linkwatch_do_dev+0x66/0xc8)
> [ 71.054983] ([<0000000000765fd6>] __linkwatch_run_queue+0x13e/0x190)
> [ 71.054985] ([<0000000000766070>] linkwatch_event+0x48/0x58)
> [ 71.054988] ([<0000000000162a2e>] process_one_work+0x3fe/0x820)
> [ 71.054990] ([<00000000001630e6>] worker_thread+0x296/0x460)
> [ 71.054992] ([<000000000016b41a>] kthread+0x112/0x120)
> [ 71.054996] ([<00000000008762b2>] kernel_thread_starter+0x6/0xc)
> [ 71.054998] ([<00000000008762ac>] kernel_thread_starter+0x0/0xc)
> [ 71.055000] INFO: lockdep is turned off.
> [ 71.055001] Last Breaking-Event-Address:
> [ 71.055004] [<0000000000294480>] printk+0xc8/0xd0
> [ 71.055006]
> [ 71.055008] Kernel panic - not syncing: Fatal exception: panic_on_oops
>
>
> This was observed with 4.8 but it's also reproducible on 4.9-rc1.
> In mlx4_en_get_phys_port_id (which looks like it's called from userspace
> via sysfs) the data behind mlx4_en_priv->mdev is already freed.
>
> The problem probably is that the lifetime of mlx4_en_priv->mdev seems to
> be shorter than that of struct net_device (and mlx4_en_get_phys_port_id
> can be called as long as struct net_device exists).
Right. This happens because we've already freed some resources.
One possible solution is to add a check of netif_device_present in
dev_get_phys_port_id.
Something like this:
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6601,6 +6601,8 @@ int dev_get_phys_port_id(struct net_device *dev,
if (!ops->ndo_get_phys_port_id)
return -EOPNOTSUPP;
+ if (!netif_device_present(dev))
+ return -ENODEV;
return ops->ndo_get_phys_port_id(dev, ppid);
}
EXPORT_SYMBOL(dev_get_phys_port_id);
However, this causes other issues when combining with MTU change.
In MTU change, netif_device_present returns false for a while, causing
an unexpected failure of dev_get_phys_port_id.
>
> Regards,
> Sebastian
>
Regards,
Tariq Toukan
^ permalink raw reply
* [PATCH net-next] net: phy: aquantia: add PHY ID of AQR106 and AQR107
From: shh.xie @ 2016-10-20 8:30 UTC (permalink / raw)
To: netdev, davem; +Cc: f.fainelli, Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@nxp.com>
The AQR106 and AQR107 can use the existing driver.
Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
---
drivers/net/phy/aquantia.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
index 09b0b0a..e8ae50e 100644
--- a/drivers/net/phy/aquantia.c
+++ b/drivers/net/phy/aquantia.c
@@ -21,6 +21,8 @@
#define PHY_ID_AQ1202 0x03a1b445
#define PHY_ID_AQ2104 0x03a1b460
#define PHY_ID_AQR105 0x03a1b4a2
+#define PHY_ID_AQR106 0x03a1b4d0
+#define PHY_ID_AQR107 0x03a1b4e0
#define PHY_ID_AQR405 0x03a1b4b0
#define PHY_AQUANTIA_FEATURES (SUPPORTED_10000baseT_Full | \
@@ -154,6 +156,30 @@ static struct phy_driver aquantia_driver[] = {
.read_status = aquantia_read_status,
},
{
+ .phy_id = PHY_ID_AQR106,
+ .phy_id_mask = 0xfffffff0,
+ .name = "Aquantia AQR106",
+ .features = PHY_AQUANTIA_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .aneg_done = aquantia_aneg_done,
+ .config_aneg = aquantia_config_aneg,
+ .config_intr = aquantia_config_intr,
+ .ack_interrupt = aquantia_ack_interrupt,
+ .read_status = aquantia_read_status,
+},
+{
+ .phy_id = PHY_ID_AQR107,
+ .phy_id_mask = 0xfffffff0,
+ .name = "Aquantia AQR107",
+ .features = PHY_AQUANTIA_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .aneg_done = aquantia_aneg_done,
+ .config_aneg = aquantia_config_aneg,
+ .config_intr = aquantia_config_intr,
+ .ack_interrupt = aquantia_ack_interrupt,
+ .read_status = aquantia_read_status,
+},
+{
.phy_id = PHY_ID_AQR405,
.phy_id_mask = 0xfffffff0,
.name = "Aquantia AQR405",
@@ -173,6 +199,8 @@ static struct mdio_device_id __maybe_unused aquantia_tbl[] = {
{ PHY_ID_AQ1202, 0xfffffff0 },
{ PHY_ID_AQ2104, 0xfffffff0 },
{ PHY_ID_AQR105, 0xfffffff0 },
+ { PHY_ID_AQR106, 0xfffffff0 },
+ { PHY_ID_AQR107, 0xfffffff0 },
{ PHY_ID_AQR405, 0xfffffff0 },
{ }
};
--
2.1.0.27.g96db324
^ permalink raw reply related
* Re: [Patch net v2] ipv6: fix a potential deadlock in do_ipv6_setsockopt()
From: Marcelo Ricardo Leitner @ 2016-10-20 11:12 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, eric.dumazet, sploving1
In-Reply-To: <1476945312-15572-1-git-send-email-xiyou.wangcong@gmail.com>
On Wed, Oct 19, 2016 at 11:35:12PM -0700, Cong Wang wrote:
> Baozeng reported this deadlock case:
>
> CPU0 CPU1
> ---- ----
> lock([ 165.136033] sk_lock-AF_INET6);
> lock([ 165.136033] rtnl_mutex);
> lock([ 165.136033] sk_lock-AF_INET6);
> lock([ 165.136033] rtnl_mutex);
>
> Similar to commit 87e9f0315952
> ("ipv4: fix a potential deadlock in mcast getsockopt() path")
> this is due to we still have a case, ipv6_sock_mc_close(),
> where we acquire sk_lock before rtnl_lock. Close this deadlock
> with the similar solution, that is always acquire rtnl lock first.
>
> Fixes: baf606d9c9b1 ("ipv4,ipv6: grab rtnl before locking the socket")
> Reported-by: Baozeng Ding <sploving1@gmail.com>
> Tested-by: Baozeng Ding <sploving1@gmail.com>
> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> include/net/addrconf.h | 1 +
> net/ipv6/ipv6_sockglue.c | 3 ++-
> net/ipv6/mcast.c | 17 ++++++++++++-----
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index f2d0727..8f998af 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -174,6 +174,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex,
> const struct in6_addr *addr);
> int ipv6_sock_mc_drop(struct sock *sk, int ifindex,
> const struct in6_addr *addr);
> +void __ipv6_sock_mc_close(struct sock *sk);
> void ipv6_sock_mc_close(struct sock *sk);
> bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
> const struct in6_addr *src_addr);
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 5330262..636ec56 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -120,6 +120,7 @@ struct ipv6_txoptions *ipv6_update_options(struct sock *sk,
> static bool setsockopt_needs_rtnl(int optname)
> {
> switch (optname) {
> + case IPV6_ADDRFORM:
> case IPV6_ADD_MEMBERSHIP:
> case IPV6_DROP_MEMBERSHIP:
> case IPV6_JOIN_ANYCAST:
> @@ -198,7 +199,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> }
>
> fl6_free_socklist(sk);
> - ipv6_sock_mc_close(sk);
> + __ipv6_sock_mc_close(sk);
Considering we already took rtnl lock and the way __ipv6_sock_mc_close()
is written, we don't need that check
if (!rcu_access_pointer(np->ipv6_mc_list))
here too as the while() in there does it already.
LGTM, thanks
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> /*
> * Sock is moving from IPv6 to IPv4 (sk_prot), so
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 75c1fc5..14a3903 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -276,16 +276,14 @@ static struct inet6_dev *ip6_mc_find_dev_rcu(struct net *net,
> return idev;
> }
>
> -void ipv6_sock_mc_close(struct sock *sk)
> +void __ipv6_sock_mc_close(struct sock *sk)
> {
> struct ipv6_pinfo *np = inet6_sk(sk);
> struct ipv6_mc_socklist *mc_lst;
> struct net *net = sock_net(sk);
>
> - if (!rcu_access_pointer(np->ipv6_mc_list))
> - return;
> + ASSERT_RTNL();
>
> - rtnl_lock();
> while ((mc_lst = rtnl_dereference(np->ipv6_mc_list)) != NULL) {
> struct net_device *dev;
>
> @@ -303,8 +301,17 @@ void ipv6_sock_mc_close(struct sock *sk)
>
> atomic_sub(sizeof(*mc_lst), &sk->sk_omem_alloc);
> kfree_rcu(mc_lst, rcu);
> -
> }
> +}
> +
> +void ipv6_sock_mc_close(struct sock *sk)
> +{
> + struct ipv6_pinfo *np = inet6_sk(sk);
> +
> + if (!rcu_access_pointer(np->ipv6_mc_list))
> + return;
> + rtnl_lock();
> + __ipv6_sock_mc_close(sk);
> rtnl_unlock();
> }
>
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH] ipv6: properly prevent temp_prefered_lft sysctl race
From: Jiri Bohac @ 2016-10-20 10:29 UTC (permalink / raw)
To: David Miller
Cc: julia.lawall, kuznet, jmorris, yoshfuji, kaber, netdev,
kbuild-all
In-Reply-To: <20161019.145816.259194714319437266.davem@davemloft.net>
The check for an underflow of tmp_prefered_lft is always false
because tmp_prefered_lft is unsigned. The intention of the check
was to guard against racing with an update of the
temp_prefered_lft sysctl, potentially resulting in an underflow.
As suggested by David Miller, the best way to prevent the race is
by reading the sysctl variable using READ_ONCE.
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
Fixes: 76506a986dc3 ("IPv6: fix DESYNC_FACTOR")
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cc7c26d..060dd99 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1185,6 +1185,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
u32 addr_flags;
unsigned long now = jiffies;
long max_desync_factor;
+ s32 cnf_temp_preferred_lft;
write_lock_bh(&idev->lock);
if (ift) {
@@ -1228,9 +1229,10 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
/* recalculate max_desync_factor each time and update
* idev->desync_factor if it's larger
*/
+ cnf_temp_preferred_lft = READ_ONCE(idev->cnf.temp_prefered_lft);
max_desync_factor = min_t(__u32,
idev->cnf.max_desync_factor,
- idev->cnf.temp_prefered_lft - regen_advance);
+ cnf_temp_preferred_lft - regen_advance);
if (unlikely(idev->desync_factor > max_desync_factor)) {
if (max_desync_factor > 0) {
@@ -1245,11 +1247,8 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
tmp_valid_lft = min_t(__u32,
ifp->valid_lft,
idev->cnf.temp_valid_lft + age);
- tmp_prefered_lft = idev->cnf.temp_prefered_lft + age -
+ tmp_prefered_lft = cnf_temp_preferred_lft + age -
idev->desync_factor;
- /* guard against underflow in case of concurrent updates to cnf */
- if (unlikely(tmp_prefered_lft < 0))
- tmp_prefered_lft = 0;
tmp_prefered_lft = min_t(__u32, ifp->prefered_lft, tmp_prefered_lft);
tmp_plen = ifp->prefix_len;
tmp_tstamp = ifp->tstamp;
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
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