* Re: [RFC PATCH net-next 04/11] spi: spi-fsl-dspi: Cosmetic cleanup
From: Vladimir Oltean @ 2019-08-17 10:44 UTC (permalink / raw)
To: Mark Brown
Cc: Hubert Feurstein, mlichvar, Richard Cochran, Andrew Lunn,
Florian Fainelli, linux-spi, netdev, David S. Miller
In-Reply-To: <20190816125942.GG4039@sirena.co.uk>
On Fri, 16 Aug 2019 at 15:59, Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Aug 16, 2019 at 03:37:46PM +0300, Vladimir Oltean wrote:
> > On Fri, 16 Aug 2019 at 15:21, Mark Brown <broonie@kernel.org> wrote:
>
> > > This is difficult to review since there's a bunch of largely unrelated
> > > changes all munged into one patch. It'd be better to split this up so
> > > each change makes one kind of fix, and better to do this separately to
> > > the rest of the series. In particular having alignment changes along
> > > with other changes hurts reviewability as it's less immediately clear
> > > what's a like for liken substitution.
>
> > Yes, the diff of this patch looks relatively bad. But I don't know if
> > splitting it in more patches isn't in fact going to pollute the git
> > history, so I can just as well drop it.
>
> No problem with lots of patches in git history if you want to split it
> up (and probably split it out of the series). Like I say it's mainly
> the alignment changes that it'd be better to pull out, the others really
> should be but it's easier to cope there.
Yes, normally it would make sense to pull these out of the patchset.
But basically all the future patches I plan to send to net-next for
this release somehow depend on this dspi driver rework.
My plan was that once the patchset reaches a stage where you accept
it, to ask Dave M. to temporarily pull the series into net-next as
well, so that the tree compiles and I can continue to work on other
sja1105 stuff. He can then drop it during the merge window. From that
perspective, even if the entire series takes more time to get accepted
rather than individual bits, at least there would be 1 single patchset
for Dave to pull.
Let me know if there's a better way to handle this.
Thanks,
-Vladimir
^ permalink raw reply
* [PATCH net-next v3 3/3] net: phy: remove genphy_config_init
From: Heiner Kallweit @ 2019-08-17 10:30 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
Vivien Didelot
Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <8790db9d-af10-c3b1-bc65-ee21bb99e6d9@gmail.com>
Now that all users have been removed we can remove genphy_config_init.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy_device.c | 51 ------------------------------------
include/linux/phy.h | 1 -
2 files changed, 52 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9c546bae9..d5db7604d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1885,57 +1885,6 @@ int genphy_soft_reset(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_soft_reset);
-int genphy_config_init(struct phy_device *phydev)
-{
- int val;
- __ETHTOOL_DECLARE_LINK_MODE_MASK(features) = { 0, };
-
- linkmode_set_bit_array(phy_basic_ports_array,
- ARRAY_SIZE(phy_basic_ports_array),
- features);
- linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, features);
- linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, features);
-
- /* Do we support autonegotiation? */
- val = phy_read(phydev, MII_BMSR);
- if (val < 0)
- return val;
-
- if (val & BMSR_ANEGCAPABLE)
- linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, features);
-
- if (val & BMSR_100FULL)
- linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, features);
- if (val & BMSR_100HALF)
- linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, features);
- if (val & BMSR_10FULL)
- linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, features);
- if (val & BMSR_10HALF)
- linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, features);
-
- if (val & BMSR_ESTATEN) {
- val = phy_read(phydev, MII_ESTATUS);
- if (val < 0)
- return val;
-
- if (val & ESTATUS_1000_TFULL)
- linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
- features);
- if (val & ESTATUS_1000_THALF)
- linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
- features);
- if (val & ESTATUS_1000_XFULL)
- linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
- features);
- }
-
- linkmode_and(phydev->supported, phydev->supported, features);
- linkmode_and(phydev->advertising, phydev->advertising, features);
-
- return 0;
-}
-EXPORT_SYMBOL(genphy_config_init);
-
/**
* genphy_read_abilities - read PHY abilities from Clause 22 registers
* @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5ac7d2137..d26779f1f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1069,7 +1069,6 @@ void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
void phy_attached_info(struct phy_device *phydev);
/* Clause 22 PHY */
-int genphy_config_init(struct phy_device *phydev);
int genphy_read_abilities(struct phy_device *phydev);
int genphy_setup_forced(struct phy_device *phydev);
int genphy_restart_aneg(struct phy_device *phydev);
--
2.22.1
^ permalink raw reply related
* [PATCH net-next v3 2/3] net: dsa: remove calls to genphy_config_init
From: Heiner Kallweit @ 2019-08-17 10:29 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
Vivien Didelot
Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <8790db9d-af10-c3b1-bc65-ee21bb99e6d9@gmail.com>
Supported PHY features are either auto-detected or explicitly set.
In both cases calling genphy_config_init isn't needed.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
net/dsa/port.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index f071acf28..f75301456 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -538,10 +538,6 @@ static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable)
return PTR_ERR(phydev);
if (enable) {
- err = genphy_config_init(phydev);
- if (err < 0)
- goto err_put_dev;
-
err = genphy_resume(phydev);
if (err < 0)
goto err_put_dev;
@@ -589,7 +585,6 @@ static int dsa_port_fixed_link_register_of(struct dsa_port *dp)
mode = PHY_INTERFACE_MODE_NA;
phydev->interface = mode;
- genphy_config_init(phydev);
genphy_read_status(phydev);
if (ds->ops->adjust_link)
--
2.22.1
^ permalink raw reply related
* [PATCH net-next v3 1/3] net: phy: remove calls to genphy_config_init
From: Heiner Kallweit @ 2019-08-17 10:29 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
Vivien Didelot
Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <8790db9d-af10-c3b1-bc65-ee21bb99e6d9@gmail.com>
Supported PHY features are either auto-detected or explicitly set.
In both cases calling genphy_config_init isn't needed. All that
genphy_config_init does is removing features that are set as
supported but can't be auto-detected. Basically it duplicates the
code in genphy_read_abilities. Therefore remove such calls from
all PHY drivers.
v2:
- remove call also from new adin PHY driver
v3:
- pass NULL as config_init function pointer for dp83848
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/adin.c | 4 ----
drivers/net/phy/at803x.c | 4 ----
drivers/net/phy/dp83822.c | 5 -----
drivers/net/phy/dp83848.c | 11 +++--------
drivers/net/phy/dp83tc811.c | 4 ----
drivers/net/phy/meson-gxl.c | 2 +-
drivers/net/phy/microchip.c | 1 -
drivers/net/phy/microchip_t1.c | 1 -
drivers/net/phy/mscc.c | 4 ++--
drivers/net/phy/vitesse.c | 6 +++---
10 files changed, 9 insertions(+), 33 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index ac79e16cd..4dec83df0 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -356,10 +356,6 @@ static int adin_config_init(struct phy_device *phydev)
phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
- rc = genphy_config_init(phydev);
- if (rc < 0)
- return rc;
-
rc = adin_config_rgmii_mode(phydev);
if (rc < 0)
return rc;
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 222ccd9ec..d98aa5671 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -249,10 +249,6 @@ static int at803x_config_init(struct phy_device *phydev)
{
int ret;
- ret = genphy_config_init(phydev);
- if (ret < 0)
- return ret;
-
/* The RX and TX delay default is:
* after HW reset: RX delay enabled and TX delay disabled
* after SW reset: RX delay enabled, while TX delay retains the
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 7ed4760fb..8a4b1d167 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -254,13 +254,8 @@ static int dp83822_config_intr(struct phy_device *phydev)
static int dp83822_config_init(struct phy_device *phydev)
{
- int err;
int value;
- err = genphy_config_init(phydev);
- if (err < 0)
- return err;
-
value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 6f9bc7d91..54c7c1b44 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -68,13 +68,8 @@ static int dp83848_config_intr(struct phy_device *phydev)
static int dp83848_config_init(struct phy_device *phydev)
{
- int err;
int val;
- err = genphy_config_init(phydev);
- if (err < 0)
- return err;
-
/* DP83620 always reports Auto Negotiation Ability on BMSR. Instead,
* we check initial value of BMCR Auto negotiation enable bit
*/
@@ -113,13 +108,13 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
static struct phy_driver dp83848_driver[] = {
DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY",
- genphy_config_init),
+ NULL),
DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY",
- genphy_config_init),
+ NULL),
DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY",
dp83848_config_init),
DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY",
- genphy_config_init),
+ NULL),
};
module_phy_driver(dp83848_driver);
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index ac27da168..06f08832e 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -277,10 +277,6 @@ static int dp83811_config_init(struct phy_device *phydev)
{
int value, err;
- err = genphy_config_init(phydev);
- if (err < 0)
- return err;
-
value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index fa80d6dce..e8f2ca625 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -136,7 +136,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
if (ret)
return ret;
- return genphy_config_init(phydev);
+ return 0;
}
/* This function is provided to cope with the possible failures of this phy
diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index eb1b3287f..a644e8e50 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -305,7 +305,6 @@ static int lan88xx_config_init(struct phy_device *phydev)
{
int val;
- genphy_config_init(phydev);
/*Zerodetect delay enable */
val = phy_read_mmd(phydev, MDIO_MMD_PCS,
PHY_ARDENNES_MMD_DEV_3_PHY_CFG);
diff --git a/drivers/net/phy/microchip_t1.c b/drivers/net/phy/microchip_t1.c
index 3d09b4716..001def450 100644
--- a/drivers/net/phy/microchip_t1.c
+++ b/drivers/net/phy/microchip_t1.c
@@ -48,7 +48,6 @@ static struct phy_driver microchip_t1_phy_driver[] = {
.features = PHY_BASIC_T1_FEATURES,
- .config_init = genphy_config_init,
.config_aneg = genphy_config_aneg,
.ack_interrupt = lan87xx_phy_ack_interrupt,
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 645d354ff..7ada1fd9c 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -1725,7 +1725,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
return ret;
}
- return genphy_config_init(phydev);
+ return 0;
err:
mutex_unlock(&phydev->mdio.bus->mdio_lock);
@@ -1767,7 +1767,7 @@ static int vsc85xx_config_init(struct phy_device *phydev)
return rc;
}
- return genphy_config_init(phydev);
+ return 0;
}
static int vsc8584_did_interrupt(struct phy_device *phydev)
diff --git a/drivers/net/phy/vitesse.c b/drivers/net/phy/vitesse.c
index 43691b1ac..bb6803527 100644
--- a/drivers/net/phy/vitesse.c
+++ b/drivers/net/phy/vitesse.c
@@ -197,7 +197,7 @@ static int vsc738x_config_init(struct phy_device *phydev)
vsc73xx_config_init(phydev);
- return genphy_config_init(phydev);
+ return 0;
}
static int vsc739x_config_init(struct phy_device *phydev)
@@ -229,7 +229,7 @@ static int vsc739x_config_init(struct phy_device *phydev)
vsc73xx_config_init(phydev);
- return genphy_config_init(phydev);
+ return 0;
}
static int vsc73xx_config_aneg(struct phy_device *phydev)
@@ -267,7 +267,7 @@ static int vsc8601_config_init(struct phy_device *phydev)
if (ret < 0)
return ret;
- return genphy_config_init(phydev);
+ return 0;
}
static int vsc824x_ack_interrupt(struct phy_device *phydev)
--
2.22.1
^ permalink raw reply related
* [PATCH net-next v3 0/3] net: phy: remove genphy_config_init
From: Heiner Kallweit @ 2019-08-17 10:28 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller, Kevin Hilman,
Vivien Didelot
Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
Supported PHY features are either auto-detected or explicitly set.
In both cases calling genphy_config_init isn't needed. All that
genphy_config_init does is removing features that are set as
supported but can't be auto-detected. Basically it duplicates the
code in genphy_read_abilities. Therefore remove genphy_config_init.
v2:
- remove call also from new adin driver
v3:
- pass NULL as config_init function pointer for dp83848
Heiner Kallweit (3):
net: phy: remove calls to genphy_config_init
net: dsa: remove calls to genphy_config_init
net: phy: remove genphy_config_init
drivers/net/phy/adin.c | 4 ---
drivers/net/phy/at803x.c | 4 ---
drivers/net/phy/dp83822.c | 5 ----
drivers/net/phy/dp83848.c | 11 ++------
drivers/net/phy/dp83tc811.c | 4 ---
drivers/net/phy/meson-gxl.c | 2 +-
drivers/net/phy/microchip.c | 1 -
drivers/net/phy/microchip_t1.c | 1 -
drivers/net/phy/mscc.c | 4 +--
drivers/net/phy/phy_device.c | 51 ----------------------------------
drivers/net/phy/vitesse.c | 6 ++--
include/linux/phy.h | 1 -
net/dsa/port.c | 5 ----
13 files changed, 9 insertions(+), 90 deletions(-)
--
2.22.1
^ permalink raw reply
* Re: [PATCH net-next v2 1/3] net: phy: remove calls to genphy_config_init
From: Heiner Kallweit @ 2019-08-17 10:25 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller, Kevin Hilman,
Vivien Didelot
Cc: netdev@vger.kernel.org, open list:ARM/Amlogic Meson...
In-Reply-To: <cc12c859-2572-02f9-3303-6a8bffad0a96@gmail.com>
On 16.08.2019 23:58, Florian Fainelli wrote:
> On 8/16/19 1:31 PM, Heiner Kallweit wrote:
>> Supported PHY features are either auto-detected or explicitly set.
>> In both cases calling genphy_config_init isn't needed. All that
>> genphy_config_init does is removing features that are set as
>> supported but can't be auto-detected. Basically it duplicates the
>> code in genphy_read_abilities. Therefore remove such calls from
>> all PHY drivers.
>>
>> v2:
>> - remove call also from new adin PHY driver
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> Looks good, just one question below:
>
>> +static int dummy_config_init(struct phy_device *phydev)
>> +{
>> + return 0;
>> +}
>> +
>> static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
>> { TI_DP83848C_PHY_ID, 0xfffffff0 },
>> { NS_DP83848C_PHY_ID, 0xfffffff0 },
>> @@ -113,13 +113,13 @@ MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
>>
>> static struct phy_driver dp83848_driver[] = {
>> DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY",
>> - genphy_config_init),
>> + dummy_config_init),
>> DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY",
>> - genphy_config_init),
>> + dummy_config_init),
>> DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY",
>> dp83848_config_init),
>> DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY",
>> - genphy_config_init),
>> + dummy_config_init),
>
> drv->config_init is an optional callback so you could just either pass
> NULL as an argument to the macro, or simply remove that parameter?
>
Yes, this can be simplified. Let's pass NULL. Thanks!
^ permalink raw reply
* Re: [PATCH net 6/6] bnxt_en: Fix to include flow direction in L2 key
From: kbuild test robot @ 2019-08-17 8:49 UTC (permalink / raw)
To: Michael Chan; +Cc: kbuild-all, davem, netdev, Somnath Kotur
In-Reply-To: <1565994817-6328-7-git-send-email-michael.chan@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 8510 bytes --]
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Michael-Chan/bnxt_en-Bug-fixes/20190817-155755
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c: In function 'bnxt_tc_get_decap_handle':
>> drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:1047:9: warning: braces around scalar initializer
struct bnxt_tc_l2_key l2_info = { {0} };
^~~~~~~~~~~~~~
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:1047:9: note: (near initialization for 'l2_info.dir')
vim +1047 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
8c95f773b4a367 Sathya Perla 2017-10-26 1040
8c95f773b4a367 Sathya Perla 2017-10-26 1041 static int bnxt_tc_get_decap_handle(struct bnxt *bp, struct bnxt_tc_flow *flow,
8c95f773b4a367 Sathya Perla 2017-10-26 1042 struct bnxt_tc_flow_node *flow_node,
8c95f773b4a367 Sathya Perla 2017-10-26 1043 __le32 *decap_filter_handle)
8c95f773b4a367 Sathya Perla 2017-10-26 1044 {
8c95f773b4a367 Sathya Perla 2017-10-26 1045 struct ip_tunnel_key *decap_key = &flow->tun_key;
cd66358e52f745 Sathya Perla 2017-10-26 1046 struct bnxt_tc_info *tc_info = bp->tc_info;
8c95f773b4a367 Sathya Perla 2017-10-26 @1047 struct bnxt_tc_l2_key l2_info = { {0} };
8c95f773b4a367 Sathya Perla 2017-10-26 1048 struct bnxt_tc_tunnel_node *decap_node;
8c95f773b4a367 Sathya Perla 2017-10-26 1049 struct ip_tunnel_key tun_key = { 0 };
8c95f773b4a367 Sathya Perla 2017-10-26 1050 struct bnxt_tc_l2_key *decap_l2_info;
8c95f773b4a367 Sathya Perla 2017-10-26 1051 __le32 ref_decap_handle;
8c95f773b4a367 Sathya Perla 2017-10-26 1052 int rc;
8c95f773b4a367 Sathya Perla 2017-10-26 1053
8c95f773b4a367 Sathya Perla 2017-10-26 1054 /* Check if there's another flow using the same tunnel decap.
8c95f773b4a367 Sathya Perla 2017-10-26 1055 * If not, add this tunnel to the table and resolve the other
479ca3bf91da97 Sriharsha Basavapatna 2018-04-11 1056 * tunnel header fileds. Ignore src_port in the tunnel_key,
479ca3bf91da97 Sriharsha Basavapatna 2018-04-11 1057 * since it is not required for decap filters.
8c95f773b4a367 Sathya Perla 2017-10-26 1058 */
479ca3bf91da97 Sriharsha Basavapatna 2018-04-11 1059 decap_key->tp_src = 0;
8c95f773b4a367 Sathya Perla 2017-10-26 1060 decap_node = bnxt_tc_get_tunnel_node(bp, &tc_info->decap_table,
8c95f773b4a367 Sathya Perla 2017-10-26 1061 &tc_info->decap_ht_params,
8c95f773b4a367 Sathya Perla 2017-10-26 1062 decap_key);
8c95f773b4a367 Sathya Perla 2017-10-26 1063 if (!decap_node)
8c95f773b4a367 Sathya Perla 2017-10-26 1064 return -ENOMEM;
8c95f773b4a367 Sathya Perla 2017-10-26 1065
8c95f773b4a367 Sathya Perla 2017-10-26 1066 flow_node->decap_node = decap_node;
8c95f773b4a367 Sathya Perla 2017-10-26 1067
8c95f773b4a367 Sathya Perla 2017-10-26 1068 if (decap_node->tunnel_handle != INVALID_TUNNEL_HANDLE)
8c95f773b4a367 Sathya Perla 2017-10-26 1069 goto done;
8c95f773b4a367 Sathya Perla 2017-10-26 1070
8c95f773b4a367 Sathya Perla 2017-10-26 1071 /* Resolve the L2 fields for tunnel decap
8c95f773b4a367 Sathya Perla 2017-10-26 1072 * Resolve the route for remote vtep (saddr) of the decap key
8c95f773b4a367 Sathya Perla 2017-10-26 1073 * Find it's next-hop mac addrs
8c95f773b4a367 Sathya Perla 2017-10-26 1074 */
8c95f773b4a367 Sathya Perla 2017-10-26 1075 tun_key.u.ipv4.dst = flow->tun_key.u.ipv4.src;
8c95f773b4a367 Sathya Perla 2017-10-26 1076 tun_key.tp_dst = flow->tun_key.tp_dst;
e9ecc731a87912 Sathya Perla 2017-12-01 1077 rc = bnxt_tc_resolve_tunnel_hdrs(bp, &tun_key, &l2_info);
8c95f773b4a367 Sathya Perla 2017-10-26 1078 if (rc)
8c95f773b4a367 Sathya Perla 2017-10-26 1079 goto put_decap;
8c95f773b4a367 Sathya Perla 2017-10-26 1080
8c95f773b4a367 Sathya Perla 2017-10-26 1081 decap_l2_info = &decap_node->l2_info;
c8fb7b8259c67b Sunil Challa 2017-12-01 1082 /* decap smac is wildcarded */
8c95f773b4a367 Sathya Perla 2017-10-26 1083 ether_addr_copy(decap_l2_info->dmac, l2_info.smac);
8c95f773b4a367 Sathya Perla 2017-10-26 1084 if (l2_info.num_vlans) {
8c95f773b4a367 Sathya Perla 2017-10-26 1085 decap_l2_info->num_vlans = l2_info.num_vlans;
8c95f773b4a367 Sathya Perla 2017-10-26 1086 decap_l2_info->inner_vlan_tpid = l2_info.inner_vlan_tpid;
8c95f773b4a367 Sathya Perla 2017-10-26 1087 decap_l2_info->inner_vlan_tci = l2_info.inner_vlan_tci;
8c95f773b4a367 Sathya Perla 2017-10-26 1088 }
8c95f773b4a367 Sathya Perla 2017-10-26 1089 flow->flags |= BNXT_TC_FLOW_FLAGS_TUNL_ETH_ADDRS;
8c95f773b4a367 Sathya Perla 2017-10-26 1090
8c95f773b4a367 Sathya Perla 2017-10-26 1091 /* For getting a decap_filter_handle we first need to check if
8c95f773b4a367 Sathya Perla 2017-10-26 1092 * there are any other decap flows that share the same tunnel L2
8c95f773b4a367 Sathya Perla 2017-10-26 1093 * key and if so, pass that flow's decap_filter_handle as the
8c95f773b4a367 Sathya Perla 2017-10-26 1094 * ref_decap_handle for this flow.
8c95f773b4a367 Sathya Perla 2017-10-26 1095 */
8c95f773b4a367 Sathya Perla 2017-10-26 1096 rc = bnxt_tc_get_ref_decap_handle(bp, flow, decap_l2_info, flow_node,
8c95f773b4a367 Sathya Perla 2017-10-26 1097 &ref_decap_handle);
8c95f773b4a367 Sathya Perla 2017-10-26 1098 if (rc)
8c95f773b4a367 Sathya Perla 2017-10-26 1099 goto put_decap;
8c95f773b4a367 Sathya Perla 2017-10-26 1100
8c95f773b4a367 Sathya Perla 2017-10-26 1101 /* Issue the hwrm cmd to allocate a decap filter handle */
8c95f773b4a367 Sathya Perla 2017-10-26 1102 rc = hwrm_cfa_decap_filter_alloc(bp, flow, decap_l2_info,
8c95f773b4a367 Sathya Perla 2017-10-26 1103 ref_decap_handle,
8c95f773b4a367 Sathya Perla 2017-10-26 1104 &decap_node->tunnel_handle);
8c95f773b4a367 Sathya Perla 2017-10-26 1105 if (rc)
8c95f773b4a367 Sathya Perla 2017-10-26 1106 goto put_decap_l2;
8c95f773b4a367 Sathya Perla 2017-10-26 1107
8c95f773b4a367 Sathya Perla 2017-10-26 1108 done:
8c95f773b4a367 Sathya Perla 2017-10-26 1109 *decap_filter_handle = decap_node->tunnel_handle;
8c95f773b4a367 Sathya Perla 2017-10-26 1110 return 0;
8c95f773b4a367 Sathya Perla 2017-10-26 1111
8c95f773b4a367 Sathya Perla 2017-10-26 1112 put_decap_l2:
8c95f773b4a367 Sathya Perla 2017-10-26 1113 bnxt_tc_put_decap_l2_node(bp, flow_node);
8c95f773b4a367 Sathya Perla 2017-10-26 1114 put_decap:
8c95f773b4a367 Sathya Perla 2017-10-26 1115 bnxt_tc_put_tunnel_node(bp, &tc_info->decap_table,
8c95f773b4a367 Sathya Perla 2017-10-26 1116 &tc_info->decap_ht_params,
8c95f773b4a367 Sathya Perla 2017-10-26 1117 flow_node->decap_node);
8c95f773b4a367 Sathya Perla 2017-10-26 1118 return rc;
8c95f773b4a367 Sathya Perla 2017-10-26 1119 }
8c95f773b4a367 Sathya Perla 2017-10-26 1120
:::::: The code at line 1047 was first introduced by commit
:::::: 8c95f773b4a367f7b9bcca7ab5f85675cfc812e9 bnxt_en: add support for Flower based vxlan encap/decap offload
:::::: TO: Sathya Perla <sathya.perla@broadcom.com>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58668 bytes --]
^ permalink raw reply
* Re: 5.3-rc3-ish VM crash: RIP: 0010:tcp_trim_head+0x20/0xe0
From: Sander Eikelenboom @ 2019-08-17 8:24 UTC (permalink / raw)
To: Eric Dumazet, netdev, linux-kernel
In-Reply-To: <4d803565-b716-42ab-1db8-3dcade91e939@gmail.com>
On 12/08/2019 19:56, Eric Dumazet wrote:
>
>
> On 8/12/19 2:50 PM, Sander Eikelenboom wrote:
>> L.S.,
>>
>> While testing a somewhere-after-5.3-rc3 kernel (which included the latest net merge (33920f1ec5bf47c5c0a1d2113989bdd9dfb3fae9),
>> one of my Xen VM's (which gets quite some network load) crashed.
>> See below for the stacktrace.
>>
>> Unfortunately I haven't got a clear trigger, so bisection doesn't seem to be an option at the moment.
>> I haven't encountered this on 5.2, so it seems to be an regression against 5.2.
>>
>> Any ideas ?
>>
>> --
>> Sander
>>
>>
>> [16930.653595] general protection fault: 0000 [#1] SMP NOPTI
>> [16930.653624] CPU: 0 PID: 3275 Comm: rsync Not tainted 5.3.0-rc3-20190809-doflr+ #1
>> [16930.653657] RIP: 0010:tcp_trim_head+0x20/0xe0
>> [16930.653677] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8
>> [16930.653741] RSP: 0000:ffffc90000003ad8 EFLAGS: 00010286
>> [16930.653762] RAX: fffe888005bf62c0 RBX: ffff8880115fb800 RCX: 000000008010000b
>
> crash in " mov 0x20(%rax),%eax" and RAX=fffe888005bf62c0 (not a valid kernel address)
>
> Look like one bit corruption maybe.
>
> Nothing comes to mind really between 5.2 and 53 that could explain this.
>
>> [16930.653791] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff888016b00880
>> [16930.653819] RBP: ffff888016b00880 R08: 0000000000000001 R09: 0000000000000000
>> [16930.653848] R10: ffff88800ae00800 R11: 00000000bfe632e6 R12: 00000000000005a0
>> [16930.653875] R13: 0000000000000001 R14: 00000000bfe62d46 R15: 0000000000000004
>> [16930.653913] FS: 00007fe71fe2cb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>> [16930.653943] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [16930.653965] CR2: 000055de0f3e7000 CR3: 0000000011f32000 CR4: 00000000000006f0
>> [16930.653993] Call Trace:
>> [16930.654005] <IRQ>
>> [16930.654018] tcp_ack+0xbb0/0x1230
>> [16930.654033] tcp_rcv_established+0x2e8/0x630
>> [16930.654053] tcp_v4_do_rcv+0x129/0x1d0
>> [16930.654070] tcp_v4_rcv+0xac9/0xcb0
>> [16930.654088] ip_protocol_deliver_rcu+0x27/0x1b0
>> [16930.654109] ip_local_deliver_finish+0x3f/0x50
>> [16930.654128] ip_local_deliver+0x4d/0xe0
>> [16930.654145] ? ip_protocol_deliver_rcu+0x1b0/0x1b0
>> [16930.654163] ip_rcv+0x4c/0xd0
>> [16930.654179] __netif_receive_skb_one_core+0x79/0x90
>> [16930.654200] netif_receive_skb_internal+0x2a/0xa0
>> [16930.654219] napi_gro_receive+0xe7/0x140
>> [16930.654237] xennet_poll+0x9be/0xae0
>> [16930.654254] net_rx_action+0x136/0x340
>> [16930.654271] __do_softirq+0xdd/0x2cf
>> [16930.654287] irq_exit+0x7a/0xa0
>> [16930.654304] xen_evtchn_do_upcall+0x27/0x40
>> [16930.654320] xen_hvm_callback_vector+0xf/0x20
>> [16930.654339] </IRQ>
>> [16930.654349] RIP: 0033:0x55de0d87db99
>> [16930.654364] Code: 00 00 48 89 7c 24 f8 45 39 fe 45 0f 42 fe 44 89 7c 24 f4 eb 09 0f 1f 40 00 83 e9 01 74 3e 89 f2 48 63 f8 4c 01 d2 44 38 1c 3a <75> 25 44 38 6c 3a ff 75 1e 41 0f b6 3c 24 40 38 3a 75 14 41 0f b6
>> [16930.654432] RSP: 002b:00007ffd5531eec8 EFLAGS: 00000a87 ORIG_RAX: ffffffffffffff0c
>> [16930.655004] RAX: 0000000000000002 RBX: 000055de0f3e8e50 RCX: 000000000000007f
>> [16930.655034] RDX: 000055de0f3dc2d2 RSI: 0000000000003492 RDI: 0000000000000002
>> [16930.655062] RBP: 0000000000007fff R08: 00000000000080ea R09: 00000000000001f0
>> [16930.655089] R10: 000055de0f3d8e40 R11: 0000000000000094 R12: 000055de0f3e0f2a
>> [16930.655116] R13: 0000000000000010 R14: 0000000000007f16 R15: 0000000000000080
>> [16930.655144] Modules linked in:
>> [16930.655200] ---[ end trace 533367c95501b645 ]---
>> [16930.655223] RIP: 0010:tcp_trim_head+0x20/0xe0
>> [16930.655243] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8
>> [16930.655312] RSP: 0000:ffffc90000003ad8 EFLAGS: 00010286
>> [16930.655331] RAX: fffe888005bf62c0 RBX: ffff8880115fb800 RCX: 000000008010000b
>> [16930.655360] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff888016b00880
>> [16930.655387] RBP: ffff888016b00880 R08: 0000000000000001 R09: 0000000000000000
>> [16930.655414] R10: ffff88800ae00800 R11: 00000000bfe632e6 R12: 00000000000005a0
>> [16930.655441] R13: 0000000000000001 R14: 00000000bfe62d46 R15: 0000000000000004
>> [16930.655475] FS: 00007fe71fe2cb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>> [16930.655502] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [16930.655525] CR2: 000055de0f3e7000 CR3: 0000000011f32000 CR4: 00000000000006f0
>> [16930.655553] Kernel panic - not syncing: Fatal exception in interrupt
>> [16930.655789] Kernel Offset: disabled
>>
Hi Eric,
Got another VM crash, with a slightly different stacktrace this time around.
Still networking though.
--
Sander
[112522.697498] general protection fault: 0000 [#1] SMP NOPTI
[112522.697555] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.3.0-rc4-20190812-doflr+ #1
[112522.697592] RIP: 0010:skb_shift+0x63/0x430
[112522.697608] Code: bc 00 00 00 48 03 8f c0 00 00 00 f6 41 03 08 74 07 48 83 79 28 00 75 d0 8b 8e bc 00 00 00 48 03 8e c0 00 00 00 48 85 f6 74 0a <f6> 41 03 08 0f 85 09 03 00 00 49 89 fd 8b bf bc 00 00 00 41 89 d4
[112522.697673] RSP: 0018:ffffc900000039b0 EFLAGS: 00010286
[112522.697693] RAX: 00000000000005a0 RBX: ffff8880117fb800 RCX: fffe8880117da6c0
[112522.697721] RDX: 00000000000005a0 RSI: ffff8880117fb800 RDI: ffff88800ae58000
[112522.697748] RBP: ffffc900000039e8 R08: 000000000004cfe0 R09: 00000000000005a0
[112522.697775] R10: 00000000000005a0 R11: ffff8880117fb800 R12: 0000000000000000
[112522.697803] R13: 00000000c95a98c2 R14: 0000000000000000 R15: ffff88800ae58000
[112522.697839] FS: 0000000000000000(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
[112522.697869] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[112522.697895] CR2: 00007f9210d8e078 CR3: 000000000b660000 CR4: 00000000000006f0
[112522.697925] Call Trace:
[112522.697938] <IRQ>
[112522.697951] tcp_sacktag_walk+0x2af/0x480
[112522.697967] tcp_sacktag_write_queue+0x34d/0x820
[112522.697986] ? ip_forward_options.cold.0+0x1c/0x1c
[112522.698007] tcp_ack+0xb8c/0x1230
[112522.698023] ? tcp_event_new_data_sent+0x4a/0x90
[112522.698043] tcp_rcv_established+0x14c/0x630
[112522.698064] tcp_v4_do_rcv+0x129/0x1d0
[112522.698081] tcp_v4_rcv+0xac9/0xcb0
[112522.698099] ip_protocol_deliver_rcu+0x27/0x1b0
[112522.698119] ip_local_deliver_finish+0x3f/0x50
[112522.698139] ip_local_deliver+0x4d/0xe0
[112522.698155] ? ip_protocol_deliver_rcu+0x1b0/0x1b0
[112522.698177] ip_rcv+0x4c/0xd0
[112522.698194] __netif_receive_skb_one_core+0x79/0x90
[112522.698215] netif_receive_skb_internal+0x2a/0xa0
[112522.698237] napi_gro_receive+0xe7/0x140
[112522.698255] xennet_poll+0x9be/0xae0
[112522.698271] net_rx_action+0x136/0x340
[112522.698288] __do_softirq+0xdd/0x2cf
[112522.698304] irq_exit+0x7a/0xa0
[112522.698321] xen_evtchn_do_upcall+0x27/0x40
[112522.698340] xen_hvm_callback_vector+0xf/0x20
[112522.698359] </IRQ>
[112522.698373] RIP: 0010:native_safe_halt+0xe/0x10
[112522.698392] Code: 48 8b 04 25 c0 6b 01 00 f0 80 48 02 20 48 8b 00 a8 08 75 c4 eb 80 90 90 90 90 90 90 e9 07 00 00 00 0f 00 2d 54 fb 41 00 fb f4 <c3> 90 e9 07 00 00 00 0f 00 2d 44 fb 41 00 f4 c3 90 90 41 55 41 54
[112522.699522] RSP: 0018:ffffffff82a03e90 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff0c
[112522.699552] RAX: 0001a54800000000 RBX: 0000000000000000 RCX: 0000000000000001
[112522.699580] RDX: 0000000002b9f9b6 RSI: 0000000000000087 RDI: 0000000000000000
[112522.699608] RBP: 0000000000000000 R08: 000000001eb5c3cb R09: ffffffff82a08460
[112522.699634] R10: 000000000002e46e R11: 0000000000000000 R12: 0000000000000000
[112522.699662] R13: 0000000000000000 R14: ffffffff8326e0a0 R15: 0000000000000000
[112522.699692] default_idle+0x17/0x140
[112522.699709] do_idle+0x1ee/0x210
[112522.699726] cpu_startup_entry+0x14/0x20
[112522.699743] start_kernel+0x4e9/0x50b
[112522.699760] secondary_startup_64+0xa4/0xb0
[112522.699780] Modules linked in:
[112522.699829] ---[ end trace 3b8db3603485e952 ]---
[112522.699850] RIP: 0010:skb_shift+0x63/0x430
[112522.699866] Code: bc 00 00 00 48 03 8f c0 00 00 00 f6 41 03 08 74 07 48 83 79 28 00 75 d0 8b 8e bc 00 00 00 48 03 8e c0 00 00 00 48 85 f6 74 0a <f6> 41 03 08 0f 85 09 03 00 00 49 89 fd 8b bf bc 00 00 00 41 89 d4
[112522.699938] RSP: 0018:ffffc900000039b0 EFLAGS: 00010286
[112522.699959] RAX: 00000000000005a0 RBX: ffff8880117fb800 RCX: fffe8880117da6c0
[112522.699986] RDX: 00000000000005a0 RSI: ffff8880117fb800 RDI: ffff88800ae58000
[112522.700013] RBP: ffffc900000039e8 R08: 000000000004cfe0 R09: 00000000000005a0
[112522.700041] R10: 00000000000005a0 R11: ffff8880117fb800 R12: 0000000000000000
[112522.700067] R13: 00000000c95a98c2 R14: 0000000000000000 R15: ffff88800ae58000
[112522.700111] FS: 0000000000000000(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
[112522.700140] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[112522.700165] CR2: 00007f9210d8e078 CR3: 000000000b660000 CR4: 00000000000006f0
[112522.700201] Kernel panic - not syncing: Fatal exception in interrupt
[112522.702992] Kernel Offset: disabled
^ permalink raw reply
* Re: INFO: task hung in tls_sw_release_resources_tx
From: Eric Biggers @ 2019-08-17 5:47 UTC (permalink / raw)
To: Steffen Klassert
Cc: Jakub Kicinski, syzbot, ast, aviadye, borisp, bpf, daniel,
davejwatson, davem, hdanton, john.fastabend, netdev,
syzkaller-bugs, herbert, linux-crypto
In-Reply-To: <20190816190234.2aaab5b6@cakuba.netronome.com>
[+Steffen, who is the maintainer of pcrypt]
On Fri, Aug 16, 2019 at 07:02:34PM -0700, Jakub Kicinski wrote:
> On Thu, 15 Aug 2019 11:06:00 -0700, syzbot wrote:
> > syzbot has bisected this bug to:
> >
> > commit 130b392c6cd6b2aed1b7eb32253d4920babb4891
> > Author: Dave Watson <davejwatson@fb.com>
> > Date: Wed Jan 30 21:58:31 2019 +0000
> >
> > net: tls: Add tls 1.3 support
> >
> > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=118e8dee600000
> > start commit: 6d5afe20 sctp: fix memleak in sctp_send_reset_streams
> > git tree: net
> > final crash: https://syzkaller.appspot.com/x/report.txt?x=138e8dee600000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=158e8dee600000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=a4c9e9f08e9e8960
> > dashboard link: https://syzkaller.appspot.com/bug?extid=6a9ff159672dfbb41c95
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17cb0502600000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14d5dc22600000
> >
> > Reported-by: syzbot+6a9ff159672dfbb41c95@syzkaller.appspotmail.com
> > Fixes: 130b392c6cd6 ("net: tls: Add tls 1.3 support")
> >
> > For information about bisection process see: https://goo.gl/tpsmEJ#bisection
>
> CC Herbert, linux-crypto
>
> This is got to be something in the crypto code :S
>
> The test case opens a ktls socket and back log writes to it.
> Then it opens a AF_ALG socket, binds "pcrypt(gcm(aes))" and dies.
>
> The ktls socket upon close waits for async crypto callbacks, but they
> never come. If I unset CRYPTO_USER_API_AEAD or change the alg to bind
> to "gcm(aes)" the bug does not trigger.
>
> Any suggestions?
Seeing as pcrypt is involved and this is a "task hung" bug, this is probably
caused by the recursive pcrypt deadlock, which is yet to be fixed.
See the original thread for more info:
https://groups.google.com/forum/#!msg/syzkaller-bugs/1_CXUd3gBcg/BvsRLH0lAgAJ
And the syzbot dashboard link:
https://syzkaller.appspot.com/bug?id=178f2528d10720d563091fb51dceb4cb20f75525
Let's tell syzbot this is a duplicate:
#syz dup: INFO: task hung in aead_recvmsg
Steffen, do you have any plan to fix this?
- Eric
^ permalink raw reply
* Re: [PATCH net-next] net: flow_offload: convert block_ing_cb_list to regular list type
From: Jiri Pirko @ 2019-08-17 5:27 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem, wenxu, pablo
In-Reply-To: <20190816150654.22106-1-vladbu@mellanox.com>
Fri, Aug 16, 2019 at 05:06:54PM CEST, vladbu@mellanox.com wrote:
>RCU list block_ing_cb_list is protected by rcu read lock in
>flow_block_ing_cmd() and with flow_indr_block_ing_cb_lock mutex in all
>functions that use it. However, flow_block_ing_cmd() needs to call blocking
>functions while iterating block_ing_cb_list which leads to following
>suspicious RCU usage warning:
>
>[ 401.510948] =============================
>[ 401.510952] WARNING: suspicious RCU usage
>[ 401.510993] 5.3.0-rc3+ #589 Not tainted
>[ 401.510996] -----------------------------
>[ 401.511001] include/linux/rcupdate.h:265 Illegal context switch in RCU read-side critical section!
>[ 401.511004]
> other info that might help us debug this:
>
>[ 401.511008]
> rcu_scheduler_active = 2, debug_locks = 1
>[ 401.511012] 7 locks held by test-ecmp-add-v/7576:
>[ 401.511015] #0: 00000000081d71a5 (sb_writers#4){.+.+}, at: vfs_write+0x166/0x1d0
>[ 401.511037] #1: 000000002bd338c3 (&of->mutex){+.+.}, at: kernfs_fop_write+0xef/0x1b0
>[ 401.511051] #2: 00000000c921c634 (kn->count#317){.+.+}, at: kernfs_fop_write+0xf7/0x1b0
>[ 401.511062] #3: 00000000a19cdd56 (&dev->mutex){....}, at: sriov_numvfs_store+0x6b/0x130
>[ 401.511079] #4: 000000005425fa52 (pernet_ops_rwsem){++++}, at: unregister_netdevice_notifier+0x30/0x140
>[ 401.511092] #5: 00000000c5822793 (rtnl_mutex){+.+.}, at: unregister_netdevice_notifier+0x35/0x140
>[ 401.511101] #6: 00000000c2f3507e (rcu_read_lock){....}, at: flow_block_ing_cmd+0x5/0x130
>[ 401.511115]
> stack backtrace:
>[ 401.511121] CPU: 21 PID: 7576 Comm: test-ecmp-add-v Not tainted 5.3.0-rc3+ #589
>[ 401.511124] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
>[ 401.511127] Call Trace:
>[ 401.511138] dump_stack+0x85/0xc0
>[ 401.511146] ___might_sleep+0x100/0x180
>[ 401.511154] __mutex_lock+0x5b/0x960
>[ 401.511162] ? find_held_lock+0x2b/0x80
>[ 401.511173] ? __tcf_get_next_chain+0x1d/0xb0
>[ 401.511179] ? mark_held_locks+0x49/0x70
>[ 401.511194] ? __tcf_get_next_chain+0x1d/0xb0
>[ 401.511198] __tcf_get_next_chain+0x1d/0xb0
>[ 401.511251] ? uplink_rep_async_event+0x70/0x70 [mlx5_core]
>[ 401.511261] tcf_block_playback_offloads+0x39/0x160
>[ 401.511276] tcf_block_setup+0x1b0/0x240
>[ 401.511312] ? mlx5e_rep_indr_setup_tc_cb+0xca/0x290 [mlx5_core]
>[ 401.511347] ? mlx5e_rep_indr_tc_block_unbind+0x50/0x50 [mlx5_core]
>[ 401.511359] tc_indr_block_get_and_ing_cmd+0x11b/0x1e0
>[ 401.511404] ? mlx5e_rep_indr_tc_block_unbind+0x50/0x50 [mlx5_core]
>[ 401.511414] flow_block_ing_cmd+0x7e/0x130
>[ 401.511453] ? mlx5e_rep_indr_tc_block_unbind+0x50/0x50 [mlx5_core]
>[ 401.511462] __flow_indr_block_cb_unregister+0x7f/0xf0
>[ 401.511502] mlx5e_nic_rep_netdevice_event+0x75/0xb0 [mlx5_core]
>[ 401.511513] unregister_netdevice_notifier+0xe9/0x140
>[ 401.511554] mlx5e_cleanup_rep_tx+0x6f/0xe0 [mlx5_core]
>[ 401.511597] mlx5e_detach_netdev+0x4b/0x60 [mlx5_core]
>[ 401.511637] mlx5e_vport_rep_unload+0x71/0xc0 [mlx5_core]
>[ 401.511679] esw_offloads_disable+0x5b/0x90 [mlx5_core]
>[ 401.511724] mlx5_eswitch_disable.cold+0xdf/0x176 [mlx5_core]
>[ 401.511759] mlx5_device_disable_sriov+0xab/0xb0 [mlx5_core]
>[ 401.511794] mlx5_core_sriov_configure+0xaf/0xd0 [mlx5_core]
>[ 401.511805] sriov_numvfs_store+0xf8/0x130
>[ 401.511817] kernfs_fop_write+0x122/0x1b0
>[ 401.511826] vfs_write+0xdb/0x1d0
>[ 401.511835] ksys_write+0x65/0xe0
>[ 401.511847] do_syscall_64+0x5c/0xb0
>[ 401.511857] entry_SYSCALL_64_after_hwframe+0x49/0xbe
>[ 401.511862] RIP: 0033:0x7fad892d30f8
>[ 401.511868] Code: 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 25 96 0d 00 8b 00 85 c0 75 17 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 60 c3 0f 1f 80 00 00 00 00 48 83
> ec 28 48 89
>[ 401.511871] RSP: 002b:00007ffca2a9fad8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
>[ 401.511875] RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007fad892d30f8
>[ 401.511878] RDX: 0000000000000002 RSI: 000055afeb072a90 RDI: 0000000000000001
>[ 401.511881] RBP: 000055afeb072a90 R08: 00000000ffffffff R09: 000000000000000a
>[ 401.511884] R10: 000055afeb058710 R11: 0000000000000246 R12: 0000000000000002
>[ 401.511887] R13: 00007fad893a8780 R14: 0000000000000002 R15: 00007fad893a3740
>
>To fix the described incorrect RCU usage, convert block_ing_cb_list from
>RCU list to regular list and protect it with flow_indr_block_ing_cb_lock
>mutex in flow_block_ing_cmd().
>
>Fixes: 1150ab0f1b33 ("flow_offload: support get multi-subsystem block")
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* [PATCH net] tcp: make sure EPOLLOUT wont be missed
From: Eric Dumazet @ 2019-08-17 4:26 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Soheil Hassas Yeganeh, Neal Cardwell,
Eric Dumazet, Jason Baron, Vladimir Rutsky
As Jason Baron explained in commit 790ba4566c1a ("tcp: set SOCK_NOSPACE
under memory pressure"), it is crucial we properly set SOCK_NOSPACE
when needed.
However, Jason patch had a bug, because the 'nonblocking' status
as far as sk_stream_wait_memory() is concerned is governed
by MSG_DONTWAIT flag passed at sendmsg() time :
long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
So it is very possible that tcp sendmsg() calls sk_stream_wait_memory(),
and that sk_stream_wait_memory() returns -EAGAIN with SOCK_NOSPACE
cleared, if sk->sk_sndtimeo has been set to a small (but not zero)
value.
This patch removes the 'noblock' variable since we must always
set SOCK_NOSPACE if -EAGAIN is returned.
It also renames the do_nonblock label since we might reach this
code path even if we were in blocking mode.
Fixes: 790ba4566c1a ("tcp: set SOCK_NOSPACE under memory pressure")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jason Baron <jbaron@akamai.com>
Reported-by: Vladimir Rutsky <rutsky@google.com>
---
net/core/stream.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/core/stream.c b/net/core/stream.c
index e94bb02a56295ec2db34ab423a8c7c890df0a696..4f1d4aa5fb38d989a9c81f32dfce3f31bbc1fa47 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -120,7 +120,6 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
int err = 0;
long vm_wait = 0;
long current_timeo = *timeo_p;
- bool noblock = (*timeo_p ? false : true);
DEFINE_WAIT_FUNC(wait, woken_wake_function);
if (sk_stream_memory_free(sk))
@@ -133,11 +132,8 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
goto do_error;
- if (!*timeo_p) {
- if (noblock)
- set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
- goto do_nonblock;
- }
+ if (!*timeo_p)
+ goto do_eagain;
if (signal_pending(current))
goto do_interrupted;
sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
@@ -169,7 +165,13 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
do_error:
err = -EPIPE;
goto out;
-do_nonblock:
+do_eagain:
+ /* Make sure that whenever EAGAIN is returned, EPOLLOUT event can
+ * be generated later.
+ * When TCP receives ACK packets that make room, tcp_check_space()
+ * only calls tcp_new_space() if SOCK_NOSPACE is set.
+ */
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
err = -EAGAIN;
goto out;
do_interrupted:
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* Re: [PATCH net-next 1/3] net: mdio: add support for passing a PTP system timestamp to the mii_bus driver
From: Richard Cochran @ 2019-08-17 3:30 UTC (permalink / raw)
To: Hubert Feurstein
Cc: netdev, linux-kernel, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, Vladimir Oltean, David S. Miller
In-Reply-To: <20190816163157.25314-2-h.feurstein@gmail.com>
On Fri, Aug 16, 2019 at 06:31:55PM +0200, Hubert Feurstein wrote:
>
> int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
> int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
> +int __mdiobus_write_sts(struct mii_bus *bus, int addr, u32 regnum, u16 val,
> + struct ptp_system_timestamp *sts);
>
> int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum);
> int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum);
> int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val);
> int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val);
> +int mdiobus_write_sts(struct mii_bus *bus, int addr, u32 regnum, u16 val,
> + struct ptp_system_timestamp *sts);
> +int mdiobus_write_sts_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val,
> + struct ptp_system_timestamp *sts);
Following the pattern, you have made three new global
mdiobus_write_sts() functions. However, your patch set only uses
mdiobus_write_sts_nested().
Please don't add global functions with no users. Let the first user
add them, if and when the need arises.
Thanks,
Richard
^ permalink raw reply
* [PATCH net-next] net: hns: add phy_attached_info() to the hns driver
From: Yonglong Liu @ 2019-08-17 2:21 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, linuxarm, salil.mehta, yisen.zhuang,
shiju.jose
This patch adds the call to phy_attached_info() to the hns driver
to identify which exact PHY drivers is in use.
Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 1545536..a48396d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1182,6 +1182,8 @@ int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h)
if (unlikely(ret))
return -ENODEV;
+ phy_attached_info(phy_dev);
+
return 0;
}
--
2.8.1
^ permalink raw reply related
* Re: [PATCH net] net: hns: add phy_attached_info() to the hns driver
From: Yonglong Liu @ 2019-08-17 2:17 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, linuxarm, salil.mehta, yisen.zhuang,
shiju.jose
In-Reply-To: <1566006967-1509-1-git-send-email-liuyonglong@huawei.com>
Please ignore this patch, it is not bugfix, should send to net-next.
Sorry for the noise.
On 2019/8/17 9:56, Yonglong Liu wrote:
> This patch add the call to phy_attached_info() to the hns driver
> to identify which exact PHY drivers is in use.
>
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns/hns_enet.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
> index 2235dd5..ab5118d 100644
> --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
> @@ -1182,6 +1182,8 @@ int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h)
> if (unlikely(ret))
> return -ENODEV;
>
> + phy_attached_info(phy_dev);
> +
> return 0;
> }
>
>
^ permalink raw reply
* Re: INFO: task hung in tls_sw_release_resources_tx
From: Jakub Kicinski @ 2019-08-17 2:02 UTC (permalink / raw)
To: syzbot
Cc: ast, aviadye, borisp, bpf, daniel, davejwatson, davem, hdanton,
john.fastabend, netdev, syzkaller-bugs, herbert, linux-crypto
In-Reply-To: <000000000000e75f1805902bb919@google.com>
On Thu, 15 Aug 2019 11:06:00 -0700, syzbot wrote:
> syzbot has bisected this bug to:
>
> commit 130b392c6cd6b2aed1b7eb32253d4920babb4891
> Author: Dave Watson <davejwatson@fb.com>
> Date: Wed Jan 30 21:58:31 2019 +0000
>
> net: tls: Add tls 1.3 support
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=118e8dee600000
> start commit: 6d5afe20 sctp: fix memleak in sctp_send_reset_streams
> git tree: net
> final crash: https://syzkaller.appspot.com/x/report.txt?x=138e8dee600000
> console output: https://syzkaller.appspot.com/x/log.txt?x=158e8dee600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=a4c9e9f08e9e8960
> dashboard link: https://syzkaller.appspot.com/bug?extid=6a9ff159672dfbb41c95
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17cb0502600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14d5dc22600000
>
> Reported-by: syzbot+6a9ff159672dfbb41c95@syzkaller.appspotmail.com
> Fixes: 130b392c6cd6 ("net: tls: Add tls 1.3 support")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
CC Herbert, linux-crypto
This is got to be something in the crypto code :S
The test case opens a ktls socket and back log writes to it.
Then it opens a AF_ALG socket, binds "pcrypt(gcm(aes))" and dies.
The ktls socket upon close waits for async crypto callbacks, but they
never come. If I unset CRYPTO_USER_API_AEAD or change the alg to bind
to "gcm(aes)" the bug does not trigger.
Any suggestions?
^ permalink raw reply
* [PATCH net] net: hns: add phy_attached_info() to the hns driver
From: Yonglong Liu @ 2019-08-17 1:56 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, linuxarm, salil.mehta, yisen.zhuang,
shiju.jose
This patch add the call to phy_attached_info() to the hns driver
to identify which exact PHY drivers is in use.
Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 2235dd5..ab5118d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1182,6 +1182,8 @@ int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h)
if (unlikely(ret))
return -ENODEV;
+ phy_attached_info(phy_dev);
+
return 0;
}
--
2.8.1
^ permalink raw reply related
* Re: [net] tipc: fix false detection of retransmit failures
From: David Miller @ 2019-08-16 23:27 UTC (permalink / raw)
To: tuong.t.lien; +Cc: jon.maloy, maloy, ying.xue, netdev, tipc-discussion
In-Reply-To: <20190815032408.7287-1-tuong.t.lien@dektech.com.au>
From: Tuong Lien <tuong.t.lien@dektech.com.au>
Date: Thu, 15 Aug 2019 10:24:08 +0700
> This commit eliminates the use of the link 'stale_limit' & 'prev_from'
> (besides the already removed - 'stale_cnt') variables in the detection
> of repeated retransmit failures as there is no proper way to initialize
> them to avoid a false detection, i.e. it is not really a retransmission
> failure but due to a garbage values in the variables.
>
> Instead, a jiffies variable will be added to individual skbs (like the
> way we restrict the skb retransmissions) in order to mark the first skb
> retransmit time. Later on, at the next retransmissions, the timestamp
> will be checked to see if the skb in the link transmq is "too stale",
> that is, the link tolerance time has passed, so that a link reset will
> be ordered. Note, just checking on the first skb in the queue is fine
> enough since it must be the oldest one.
> A counter is also added to keep track the actual skb retransmissions'
> number for later checking when the failure happens.
>
> The downside of this approach is that the skb->cb[] buffer is about to
> be exhausted, however it is always able to allocate another memory area
> and keep a reference to it when needed.
>
> Fixes: 77cf8edbc0e7 ("tipc: simplify stale link failure criteria")
> Reported-by: Hoang Le <hoang.h.le@dektech.com.au>
> Acked-by: Ying Xue <ying.xue@windriver.com>
> Acked-by: Jon Maloy <jon.maloy@ericsson.com>
> Signed-off-by: Tuong Lien <tuong.t.lien@dektech.com.au>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH RFC net-next 3/3] net: dsa: mv88e6xxx: setup SERDES irq also for CPU/DSA ports
From: Vivien Didelot @ 2019-08-16 23:05 UTC (permalink / raw)
To: Marek Behun; +Cc: netdev, Andrew Lunn, Vladimir Oltean, Florian Fainelli
In-Reply-To: <20190816190520.57958fde@nic.cz>
Hi Marek,
On Fri, 16 Aug 2019 19:05:20 +0200, Marek Behun <marek.behun@nic.cz> wrote:
> On Fri, 16 Aug 2019 12:25:52 -0400
> Vivien Didelot <vivien.didelot@gmail.com> wrote:
>
> > So now we have mv88e6xxx_setup_port() and mv88e6xxx_port_setup(), which both
> > setup a port, differently, at different time. This is definitely error prone.
>
> Hmm. I don't know how much of mv88e6xxx_setup_port() could be moved to
> this new port_setup(), since there are other setup functions called in
> mv88e6xxx_setup() that can possibly depend on what was done by
> mv88e6xxx_setup_port().
>
> Maybe the new DSA operations should be called .after_setup()
> and .before_teardown(), and be called just once for the whole switch,
> not for each port?
I think the DSA switch port_setup/port_teardown operations are fine, but the
idea would be that the drivers must no longer setup their ports directly
in their .setup function. So for mv88e6xxx precisely, we should rename
mv88e6xxx_setup_port to mv88e6xxx_port_setup, and move all the port-related
code from mv88e6xxx_setup into mv88e6xxx_port_setup.
Also, the DSA stack must call ds->ops->port_setup() for all ports, regardless
their type, i.e. even if their are unused.
As a reminder, *setup/*teardown are more like typical probe/remove callbacks
found in drivers, while enable/disable are a runtime thing, switching a port
on and off (think ifconfig up/down).
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH] ipvlan: set hw_enc_features like macvlan
From: David Miller @ 2019-08-16 22:58 UTC (permalink / raw)
To: wsommerfeld
Cc: petrm, jiri, idosch, daniel, yuehaibing, tglx, linmiaohe,
edumazet, maheshb, netdev, linux-kernel
In-Reply-To: <20190815001043.153874-1-wsommerfeld@google.com>
From: Bill Sommerfeld <wsommerfeld@google.com>
Date: Wed, 14 Aug 2019 17:10:43 -0700
> Allow encapsulated packets sent to tunnels layered over ipvlan to use
> offloads rather than forcing SW fallbacks.
>
> Since commit f21e5077010acda73a60 ("macvlan: add offload features for
> encapsulation"), macvlan has set dev->hw_enc_features to include
> everything in dev->features; do likewise in ipvlan.
>
> Signed-off-by: Bill Sommerfeld <wsommerfeld@google.com>
Applied to net-next.
^ permalink raw reply
* Re: regression in ath10k dma allocation
From: Tobias Klausmann @ 2019-08-16 22:42 UTC (permalink / raw)
To: Nicolin Chen
Cc: Christoph Hellwig, kvalo, davem, ath10k, linux-wireless, netdev,
linux-kernel, m.szyprowski, robin.murphy, iommu, tobias.klausmann
In-Reply-To: <20190816222506.GA24413@Asurada-Nvidia.nvidia.com>
Hi Nicolin,
On 17.08.19 00:25, Nicolin Chen wrote:
> Hi Tobias
>
> On Fri, Aug 16, 2019 at 10:16:45PM +0200, Tobias Klausmann wrote:
>>> do you have CONFIG_DMA_CMA set in your config? If not please make sure
>>> you have this commit in your testing tree, and if the problem still
>>> persists it would be a little odd and we'd have to dig deeper:
>>>
>>> commit dd3dcede9fa0a0b661ac1f24843f4a1b1317fdb6
>>> Author: Nicolin Chen <nicoleotsuka@gmail.com>
>>> Date: Wed May 29 17:54:25 2019 -0700
>>>
>>> dma-contiguous: fix !CONFIG_DMA_CMA version of dma_{alloc, free}_contiguous()
>> yes CONFIG_DMA_CMA is set (=y, see attached config), the commit you mention
>> above is included, if you have any hints how to go forward, please let me
>> know!
> For CONFIG_DMA_CMA=y, by judging the log with error code -12, I
> feel this one should work for you. Would you please check if it
> is included or try it out otherwise?
>
> dma-contiguous: do not overwrite align in dma_alloc_contiguous()
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c6622a425acd1d2f3a443cd39b490a8777b622d7
Thanks for the hint, yet the commit is included and does not fix the
problem!
Greetings,
Tobias
^ permalink raw reply
* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
From: Matthias Kaehlcke @ 2019-08-16 22:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: Pavel Machek, David S . Miller, Rob Herring, Mark Rutland,
Andrew Lunn, Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <31dc724d-77ba-3400-6abe-4cf2e3c2a20a@gmail.com>
On Fri, Aug 16, 2019 at 03:12:47PM -0700, Florian Fainelli wrote:
> On 8/16/19 2:27 PM, Matthias Kaehlcke wrote:
> > On Fri, Aug 16, 2019 at 10:13:42PM +0200, Pavel Machek wrote:
> >> On Tue 2019-08-13 12:11:47, Matthias Kaehlcke wrote:
> >>> Add a .config_led hook which is called by the PHY core when
> >>> configuration data for a PHY LED is available. Each LED can be
> >>> configured to be solid 'off, solid 'on' for certain (or all)
> >>> link speeds or to blink on RX/TX activity.
> >>>
> >>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >>
> >> THis really needs to go through the LED subsystem,
> >
> > Sorry, I used what get_maintainers.pl threw at me, I should have
> > manually cc-ed the LED list.
> >
> >> and use the same userland interfaces as the rest of the system.
> >
> > With the PHY maintainers we discussed to define a binding that is
> > compatible with that of the LED one, to have the option to integrate
> > it with the LED subsystem later. The integration itself is beyond the
> > scope of this patchset.
> >
> > The PHY LED configuration is a low priority for the project I'm
> > working on. I wanted to make an attempt to upstream it and spent
> > already significantly more time on it than planned, if integration
> > with the LED framework now is a requirement please consider this
> > series abandonded.
>
> While I have an appreciation for how hard it can be to work in a
> corporate environment while doing upstream first and working with
> virtually unbounded goals (in time or scope) due to maintainers and
> reviewers, that kind of statement can hinder your ability to establish
> trust with peers in the community as it can be read as take it or leave it.
I'm really just stating the reality here. We strongly prefer landing
patches upstream over doing custom hacks, and depending on the
priority of a given feature/sub-system and impact on schedule we can
allocate more time on it or less. In some cases/at some point a
downstream patch is just good enough.
I definitely don't intend to get a patchset landed if it isn't deemed
ready or suitable at all. In this case I just can't justify to spend
significantly more time on it. IMO it is better to be clear on this,
not to pressure maintainers to take a patch, but so people know what
to expect. This information can also help if someone comes across this
patchset in the future and wonders about its status.
btw, a birdie told me there will be a talk next week at ELC in San
Diego on how Chrome OS works with upstream, discussing pros and
cons for both the project and upstream. For those who are intersted
in the topic but can't make it to the conference, the slides are
already online and IMO have good information:
https://static.sched.com/hosted_files/ossna19/9c/ELC19_ChromeOSAndUpstream.pdf
Cheers
Matthias
^ permalink raw reply
* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
From: Doug Anderson @ 2019-08-16 22:39 UTC (permalink / raw)
To: Florian Fainelli
Cc: Matthias Kaehlcke, Pavel Machek, David S . Miller, Rob Herring,
Mark Rutland, Andrew Lunn, Heiner Kallweit, netdev, devicetree,
LKML
In-Reply-To: <31dc724d-77ba-3400-6abe-4cf2e3c2a20a@gmail.com>
Hi,
On Fri, Aug 16, 2019 at 3:12 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 8/16/19 2:27 PM, Matthias Kaehlcke wrote:
> > On Fri, Aug 16, 2019 at 10:13:42PM +0200, Pavel Machek wrote:
> >> On Tue 2019-08-13 12:11:47, Matthias Kaehlcke wrote:
> >>> Add a .config_led hook which is called by the PHY core when
> >>> configuration data for a PHY LED is available. Each LED can be
> >>> configured to be solid 'off, solid 'on' for certain (or all)
> >>> link speeds or to blink on RX/TX activity.
> >>>
> >>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >>
> >> THis really needs to go through the LED subsystem,
> >
> > Sorry, I used what get_maintainers.pl threw at me, I should have
> > manually cc-ed the LED list.
> >
> >> and use the same userland interfaces as the rest of the system.
> >
> > With the PHY maintainers we discussed to define a binding that is
> > compatible with that of the LED one, to have the option to integrate
> > it with the LED subsystem later. The integration itself is beyond the
> > scope of this patchset.
> >
> > The PHY LED configuration is a low priority for the project I'm
> > working on. I wanted to make an attempt to upstream it and spent
> > already significantly more time on it than planned, if integration
> > with the LED framework now is a requirement please consider this
> > series abandonded.
>
> While I have an appreciation for how hard it can be to work in a
> corporate environment while doing upstream first and working with
> virtually unbounded goals (in time or scope) due to maintainers and
> reviewers, that kind of statement can hinder your ability to establish
> trust with peers in the community as it can be read as take it or leave it.
You think so? I feel like Matthias is simply expressing the reality
of the situation here and I'd rather see a statement like this posted
than the series just silently dropped. Communication is good.
In general on Chrome OS we don't spent lots of time tweaking with
Ethernet and even less time tweaking with Ethernet on ARM boards where
you might need a binding like this, so it's pretty hard to justify up
the management chain spending massive amounts of resources on it. In
this case we have two existing ARM boards which we're trying to uprev
from 3.14 to 4.19 which were tweaking the Ethernet driver in some
downstream code. We thought it would be nice to try to come up with a
solution that could land upstream, which is usually what we try to do
in these cases.
Normally if there is some major architecture needed that can't fit in
the scope of a project, we would do a downstream solution for the
project and then fork off the task (maybe by a different Engineer or a
contractor) to get a solution that can land upstream. ...but in this
case it seems hard to justify because it's unlikely we would need it
again anytime remotely soon.
So I guess the alternatives to what Matthias did would have been:
A) Don't even try to upstream. Seems worse. At least this way
there's something a future person can start from and the discussion is
rolling.
B) Keep spending tons of time on something even though management
doesn't want him to. Seems worse.
C) Spend his nights and weekends working on this. Seems worse.
D) Silently stop working on it without saying "I'm going to stop". Seems worse.
...unless you have a brilliant "E)" I think what Matthias did here is
exactly right.
BTW: I'm giving a talk on this topic next week at ELC [1]. If you're
going to be there feel free to attend. ...or just read the slides if
not.
> The LED subsystem integration can definitively come in later from my 2
> cents perspective and this patch series as it stands is valuable and
> avoids inventing new bindings.
If something like this series can land and someone can later try to
make the situation better then I think that would be awesome. I don't
think Matthias is saying "I won't spin" or "I won't take feedback".
He's just expressing that he can't keep working on this indefinitely.
[1] https://ossna19.sched.com/event/PVSV/how-chrome-os-works-with-upstream-linux-douglas-anderson-google
-Doug
^ permalink raw reply
* [PATCH net 5/6] bnxt_en: Use correct src_fid to determine direction of the flow
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
To: davem; +Cc: netdev, Venkat Duvvuru
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>
From: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Direction of the flow is determined using src_fid. For an RX flow,
src_fid is PF's fid and for TX flow, src_fid is VF's fid. Direction
of the flow must be specified, when getting statistics for that flow.
Currently, for DECAP flow, direction is determined incorrectly, i.e.,
direction is initialized as TX for DECAP flow, instead of RX. Because
of which, stats are not reported for this DECAP flow, though it is
offloaded and there is traffic for that flow, resulting in flow age out.
This patch fixes the problem by determining the DECAP flow's direction
using correct fid. Set the flow direction in all cases for consistency
even if 64-bit flow handle is not used.
Fixes: abd43a13525d ("bnxt_en: Support for 64-bit flow handle.")
Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 6fe4a71..6224c30 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -1285,9 +1285,7 @@ static int bnxt_tc_add_flow(struct bnxt *bp, u16 src_fid,
goto free_node;
bnxt_tc_set_src_fid(bp, flow, src_fid);
-
- if (bp->fw_cap & BNXT_FW_CAP_OVS_64BIT_HANDLE)
- bnxt_tc_set_flow_dir(bp, flow, src_fid);
+ bnxt_tc_set_flow_dir(bp, flow, flow->src_fid);
if (!bnxt_tc_can_offload(bp, flow)) {
rc = -EOPNOTSUPP;
--
2.5.1
^ permalink raw reply related
* [PATCH net 6/6] bnxt_en: Fix to include flow direction in L2 key
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
To: davem; +Cc: netdev, Somnath Kotur
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>
From: Somnath Kotur <somnath.kotur@broadcom.com>
FW expects the driver to provide unique flow reference handles
for Tx or Rx flows. When a Tx flow and an Rx flow end up sharing
a reference handle, flow offload does not seem to work.
This could happen in the case of 2 flows having their L2 fields
wildcarded but in different direction.
Fix to incorporate the flow direction as part of the L2 key
Fixes: abd43a13525d ("bnxt_en: Support for 64-bit flow handle.")
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 4 ++--
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 6224c30..dd621f6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -1236,7 +1236,7 @@ static int __bnxt_tc_del_flow(struct bnxt *bp,
static void bnxt_tc_set_flow_dir(struct bnxt *bp, struct bnxt_tc_flow *flow,
u16 src_fid)
{
- flow->dir = (bp->pf.fw_fid == src_fid) ? BNXT_DIR_RX : BNXT_DIR_TX;
+ flow->l2_key.dir = (bp->pf.fw_fid == src_fid) ? BNXT_DIR_RX : BNXT_DIR_TX;
}
static void bnxt_tc_set_src_fid(struct bnxt *bp, struct bnxt_tc_flow *flow,
@@ -1405,7 +1405,7 @@ static void bnxt_fill_cfa_stats_req(struct bnxt *bp,
* 2. 15th bit of flow_handle must specify the flow
* direction (TX/RX).
*/
- if (flow_node->flow.dir == BNXT_DIR_RX)
+ if (flow_node->flow.l2_key.dir == BNXT_DIR_RX)
handle = CFA_FLOW_INFO_REQ_FLOW_HANDLE_DIR_RX |
CFA_FLOW_INFO_REQ_FLOW_HANDLE_MAX_MASK;
else
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.h
index ffec57d..e6373b3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.h
@@ -17,6 +17,7 @@
/* Structs used for storing the filter/actions of the TC cmd.
*/
struct bnxt_tc_l2_key {
+ u8 dir;
u8 dmac[ETH_ALEN];
u8 smac[ETH_ALEN];
__be16 inner_vlan_tpid;
@@ -98,7 +99,6 @@ struct bnxt_tc_flow {
/* flow applicable to pkts ingressing on this fid */
u16 src_fid;
- u8 dir;
#define BNXT_DIR_RX 1
#define BNXT_DIR_TX 0
struct bnxt_tc_l2_key l2_key;
--
2.5.1
^ permalink raw reply related
* [PATCH net 4/6] bnxt_en: Suppress HWRM errors for HWRM_NVM_GET_VARIABLE command
From: Michael Chan @ 2019-08-16 22:33 UTC (permalink / raw)
To: davem; +Cc: netdev, Vasundhara Volam
In-Reply-To: <1565994817-6328-1-git-send-email-michael.chan@broadcom.com>
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
For newly added NVM parameters, older firmware may not have the support.
Suppress the error message to avoid the unncessary error message which is
triggered when devlink calls the driver during initialization.
Fixes: 782a624d00fa ("bnxt_en: Add bnxt_en initial params table and register it.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 549c90d3..c05d663 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -98,10 +98,13 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
if (idx)
req->dimensions = cpu_to_le16(1);
- if (req->req_type == cpu_to_le16(HWRM_NVM_SET_VARIABLE))
+ if (req->req_type == cpu_to_le16(HWRM_NVM_SET_VARIABLE)) {
memcpy(data_addr, buf, bytesize);
-
- rc = hwrm_send_message(bp, msg, msg_len, HWRM_CMD_TIMEOUT);
+ rc = hwrm_send_message(bp, msg, msg_len, HWRM_CMD_TIMEOUT);
+ } else {
+ rc = hwrm_send_message_silent(bp, msg, msg_len,
+ HWRM_CMD_TIMEOUT);
+ }
if (!rc && req->req_type == cpu_to_le16(HWRM_NVM_GET_VARIABLE))
memcpy(buf, data_addr, bytesize);
--
2.5.1
^ 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