Re: [PATCH net-next] tls: Fix socket mem accounting error under async encryption
From: Vakul Garg
Date: Wed, 26 Sep 2018 04:19:25 +
> BTW, I noticed following build failure.
> It gets resolved after reverting d6ab93364734.
>
> CC [M] drivers/net/phy/marvell.o
> drivers/net/phy/marvell.c: In function 'm88e1121_config_aneg':
> drivers/net/phy/marvell.c:468:25: error: 'autoneg' undeclared (first use in
> this function); did you mean 'put_net'?
> if (phydev->autoneg != autoneg || changed) {
> ^~~
> put_net
> drivers/net/phy/marvell.c:468:25: note: each undeclared identifier is
> reported only once for each function it appears in
>
Thanks, I just fixed it as follows below.
Please CC: the patch author when you report build failures like this
in the future.
>From 4b1bd69769454175268908f50b32f1cbfee5bb83 Mon Sep 17 00:00:00 2001
From: "David S. Miller"
Date: Tue, 25 Sep 2018 22:41:31 -0700
Subject: [PATCH] net: phy: marvell: Fix build.
Local variable 'autoneg' doesn't even exist:
drivers/net/phy/marvell.c: In function 'm88e1121_config_aneg':
drivers/net/phy/marvell.c:468:25: error: 'autoneg' undeclared (first use in
this function); did you mean 'put_net'?
if (phydev->autoneg != autoneg || changed) {
^~~
Fixes: d6ab93364734 ("net: phy: marvell: Avoid unnecessary soft reset")
Reported-by:Vakul Garg
Signed-off-by: David S. Miller
---
drivers/net/phy/marvell.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index b55a7376bfdc..24fc4a73c300 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -465,7 +465,7 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
if (err < 0)
return err;
- if (phydev->autoneg != autoneg || changed) {
+ if (phydev->autoneg != AUTONEG_ENABLE || changed) {
/* A software reset is used to ensure a "commit" of the
* changes is done.
*/
--
2.17.1
RE: [PATCH net-next] tls: Fix socket mem accounting error under async encryption
> -Original Message-
> From: David Miller
> Sent: Wednesday, September 26, 2018 9:10 AM
> To: Vakul Garg
> Cc: netdev@vger.kernel.org; bor...@mellanox.com;
> avia...@mellanox.com; davejwat...@fb.com; doro...@fb.com
> Subject: Re: [PATCH net-next] tls: Fix socket mem accounting error under
> async encryption
>
> From: Vakul Garg
> Date: Wed, 26 Sep 2018 01:54:25 +
>
> > I don't find this patch and one other ("tls: Fixed a memory leak
> > during socket close") in linux-net-next. Could you please kindly
> > check? Regards.
>
> After applying I didn't push out and instead I started a test build, closed my
> laptop, did a lot of other things and just came back to finish the build.
>
> It'll show up momentarily.
>
> Thanks for your patience.
Thanks for explaining the workflow.
BTW, I noticed following build failure.
It gets resolved after reverting d6ab93364734.
CC [M] drivers/net/phy/marvell.o
drivers/net/phy/marvell.c: In function 'm88e1121_config_aneg':
drivers/net/phy/marvell.c:468:25: error: 'autoneg' undeclared (first use in
this function); did you mean 'put_net'?
if (phydev->autoneg != autoneg || changed) {
^~~
put_net
drivers/net/phy/marvell.c:468:25: note: each undeclared identifier is reported
only once for each function it appears in
Re: [PATCH net-next] tls: Fix socket mem accounting error under async encryption
From: Vakul Garg
Date: Wed, 26 Sep 2018 01:54:25 +
> I don't find this patch and one other ("tls: Fixed a memory leak
> during socket close") in linux-net-next. Could you please kindly
> check? Regards.
After applying I didn't push out and instead I started a test build,
closed my laptop, did a lot of other things and just came back to
finish the build.
It'll show up momentarily.
Thanks for your patience.
RE: [PATCH net-next] tls: Fix socket mem accounting error under async encryption
> -Original Message-
> From: David Miller
> Sent: Tuesday, September 25, 2018 11:14 PM
> To: Vakul Garg
> Cc: netdev@vger.kernel.org; bor...@mellanox.com;
> avia...@mellanox.com; davejwat...@fb.com; doro...@fb.com
> Subject: Re: [PATCH net-next] tls: Fix socket mem accounting error under
> async encryption
>
> From: Vakul Garg
> Date: Tue, 25 Sep 2018 16:26:17 +0530
>
> > Current async encryption implementation sometimes showed up socket
> > memory accounting error during socket close. This results in kernel
> > warning calltrace. The root cause of the problem is that socket var
> > sk_forward_alloc gets corrupted due to access in sk_mem_charge() and
> > sk_mem_uncharge() being invoked from multiple concurrent contexts in
> > multicore processor. The apis sk_mem_charge() and sk_mem_uncharge()
> > are called from functions alloc_plaintext_sg(), free_sg() etc. It is
> > required that memory accounting apis are called under a socket lock.
> >
> > The plaintext sg data sent for encryption is freed using free_sg() in
> > tls_encryption_done(). It is wrong to call free_sg() from this function.
> > This is because this function may run in irq context. We cannot
> > acquire socket lock in this function.
> >
> > We remove calling of function free_sg() for plaintext data from
> > tls_encryption_done() and defer freeing up of plaintext data to the
> > time when the record is picked up from tx_list and transmitted/freed.
> > When
> > tls_tx_records() gets called, socket is already locked and thus there
> > is no concurrent access problem.
> >
> > Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
> > Signed-off-by: Vakul Garg
>
> Applied.
I don't find this patch and one other ("tls: Fixed a memory leak during socket
close")
in linux-net-next. Could you please kindly check? Regards.
Re: [PATCH net-next] tls: Fix socket mem accounting error under async encryption
From: Vakul Garg
Date: Tue, 25 Sep 2018 16:26:17 +0530
> Current async encryption implementation sometimes showed up socket
> memory accounting error during socket close. This results in kernel
> warning calltrace. The root cause of the problem is that socket var
> sk_forward_alloc gets corrupted due to access in sk_mem_charge()
> and sk_mem_uncharge() being invoked from multiple concurrent contexts
> in multicore processor. The apis sk_mem_charge() and sk_mem_uncharge()
> are called from functions alloc_plaintext_sg(), free_sg() etc. It is
> required that memory accounting apis are called under a socket lock.
>
> The plaintext sg data sent for encryption is freed using free_sg() in
> tls_encryption_done(). It is wrong to call free_sg() from this function.
> This is because this function may run in irq context. We cannot acquire
> socket lock in this function.
>
> We remove calling of function free_sg() for plaintext data from
> tls_encryption_done() and defer freeing up of plaintext data to the time
> when the record is picked up from tx_list and transmitted/freed. When
> tls_tx_records() gets called, socket is already locked and thus there is
> no concurrent access problem.
>
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption")
> Signed-off-by: Vakul Garg
Applied.
