[PATCH] bpf: Fix backport of "bpf: restrict unknown scalars of mixed signed bounds for unprivileged"
The 4.14 backport of 9d7eceede ("bpf: restrict unknown scalars of mixed
signed bounds for unprivileged") adds the PTR_TO_MAP_VALUE check to the
wrong location in adjust_ptr_min_max_vals(), most likely because 4.14
doesn't include the commit that updates the if-statement to a
switch-statement (aad2eeaf4 "bpf: Simplify ptr_min_max_vals adjustment").
Move the check to the proper location in adjust_ptr_min_max_vals().
Fixes: 17efa65350c5a ("bpf: restrict unknown scalars of mixed signed bounds for
unprivileged")
Signed-off-by: Samuel Mendoza-Jonas
Reviewed-by: Frank van der Linden
Reviewed-by: Ethan Chen
---
kernel/bpf/verifier.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0c3a9302be93..9e9b7c076bcb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2204,6 +2204,13 @@ static int adjust_ptr_min_max_vals(struct
bpf_verifier_env *env,
dst);
return -EACCES;
}
+ if (ptr_reg->type == PTR_TO_MAP_VALUE) {
+ if (!env->allow_ptr_leaks && !known && (smin_val < 0) !=
(smax_val < 0)) {
+ verbose("R%d has unknown scalar with mixed signed
bounds, pointer arithmetic with it prohibited for !root\n",
+ off_reg == dst_reg ? dst : src);
+ return -EACCES;
+ }
+ }
/* In case of 'scalar += pointer', dst_reg inherits pointer type and id.
* The id may be overwritten later if we create a new variable offset.
@@ -2349,13 +2356,6 @@ static int adjust_ptr_min_max_vals(struct
bpf_verifier_env *env,
verbose("R%d bitwise operator %s on pointer
prohibited\n",
dst, bpf_alu_string[opcode >> 4]);
return -EACCES;
- case PTR_TO_MAP_VALUE:
- if (!env->allow_ptr_leaks && !known && (smin_val < 0) !=
(smax_val < 0)) {
- verbose("R%d has unknown scalar with mixed signed
bounds, pointer arithmetic with it prohibited for !root\n",
- off_reg == dst_reg ? dst : src);
- return -EACCES;
- }
- /* fall-through */
default:
/* other operators (e.g. MUL,LSH) produce non-pointer results */
if (!env->allow_ptr_leaks)
--
2.17.1
[PATCH net-next v2 5/5] net: stmmac: dwmac-sun8i: Add a shutdown callback
The Ethernet MAC and PHY are usually major consumers of power on boards
which may not be able to fully power off (those with no PMIC). Powering
down the MAC and internal PHY saves power while these boards are "off".
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a3d333b652836..6b75cf2603ffc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1284,6 +1284,15 @@ static int sun8i_dwmac_remove(struct platform_device
*pdev)
return 0;
}
+static void sun8i_dwmac_shutdown(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+ sun8i_dwmac_exit(pdev, gmac);
+}
+
static const struct of_device_id sun8i_dwmac_match[] = {
{ .compatible = "allwinner,sun8i-h3-emac",
.data = &emac_variant_h3 },
@@ -1304,6 +1313,7 @@ MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
static struct platform_driver sun8i_dwmac_driver = {
.probe = sun8i_dwmac_probe,
.remove = sun8i_dwmac_remove,
+ .shutdown = sun8i_dwmac_shutdown,
.driver = {
.name = "dwmac-sun8i",
.pm = &stmmac_pltfr_pm_ops,
--
2.26.2
[PATCH net-next v2 1/5] net: stmmac: dwmac-sun8i: Return void from PHY unpower
This is a deinitialization function that always returned zero, and that
return value was always ignored. Have it return void instead.
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a5e0eff4a3874..8e505019adf85 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -820,15 +820,14 @@ static int sun8i_dwmac_power_internal_phy(struct
stmmac_priv *priv)
return 0;
}
-static int sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
+static void sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
{
if (!gmac->internal_phy_powered)
- return 0;
+ return;
clk_disable_unprepare(gmac->ephy_clk);
reset_control_assert(gmac->rst_ephy);
gmac->internal_phy_powered = false;
- return 0;
}
/* MDIO multiplexing switch function
--
2.26.2
[PATCH net-next v2 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
Use the appropriate function instead of reimplementing it,
and update the error message to match the code.
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 3c3d0b99d3e8c..b61f442ed3033 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -805,12 +805,12 @@ static int sun8i_dwmac_power_internal_phy(struct
stmmac_priv *priv)
/* Make sure the EPHY is properly reseted, as U-Boot may leave
* it at deasserted state, and thus it may fail to reset EMAC.
+*
+* This assumes the driver has exclusive access to the EPHY reset.
*/
- reset_control_assert(gmac->rst_ephy);
-
- ret = reset_control_deassert(gmac->rst_ephy);
+ ret = reset_control_reset(gmac->rst_ephy);
if (ret) {
- dev_err(priv->device, "Cannot deassert internal phy\n");
+ dev_err(priv->device, "Cannot reset internal PHY\n");
clk_disable_unprepare(gmac->ephy_clk);
return ret;
}
--
2.26.2
[PATCH net-next v2 0/5] dwmac-sun8i cleanup and shutdown hook
These patches clean up some things I noticed while fixing suspend/resume behavior. The first four are minor code improvements. The last one adds a shutdown hook to minimize power consumption on boards without a PMIC. Changes v1 to v2: - Note the assumption of exclusive reset controller access in patch 3 Samuel Holland (5): net: stmmac: dwmac-sun8i: Return void from PHY unpower net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check net: stmmac: dwmac-sun8i: Use reset_control_reset net: stmmac: dwmac-sun8i: Minor probe function cleanup net: stmmac: dwmac-sun8i: Add a shutdown callback .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 33 --- 1 file changed, 21 insertions(+), 12 deletions(-) -- 2.26.2
[PATCH net-next v2 4/5] net: stmmac: dwmac-sun8i: Minor probe function cleanup
Adjust the spacing and use an explicit "return 0" in the success path to make the function easier to parse. Reviewed-by: Chen-Yu Tsai Signed-off-by: Samuel Holland --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index b61f442ed3033..a3d333b652836 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -1229,6 +1229,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) ndev = dev_get_drvdata(&pdev->dev); priv = netdev_priv(ndev); + /* The mux must be registered after parent MDIO * so after stmmac_dvr_probe() */ @@ -1247,7 +1248,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) goto dwmac_remove; } - return ret; + return 0; + dwmac_mux: reset_control_put(gmac->rst_ephy); clk_put(gmac->ephy_clk); -- 2.26.2
[PATCH net-next v2 2/5] net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
sun8i_dwmac_unpower_internal_phy already checks if the PHY is powered,
so there is no need to do it again here.
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 8e505019adf85..3c3d0b99d3e8c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1018,10 +1018,8 @@ static void sun8i_dwmac_exit(struct platform_device
*pdev, void *priv)
{
struct sunxi_priv_data *gmac = priv;
- if (gmac->variant->soc_has_internal_phy) {
- if (gmac->internal_phy_powered)
- sun8i_dwmac_unpower_internal_phy(gmac);
- }
+ if (gmac->variant->soc_has_internal_phy)
+ sun8i_dwmac_unpower_internal_phy(gmac);
clk_disable_unprepare(gmac->tx_clk);
--
2.26.2
Re: [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
On 2/8/21 10:29 AM, Alexander Duyck wrote:
> On Sun, Feb 7, 2021 at 10:32 PM Samuel Holland wrote:
>>
>> Use the appropriate function instead of reimplementing it,
>> and update the error message to match the code.
>>
>> Reviewed-by: Chen-Yu Tsai
>> Signed-off-by: Samuel Holland
>> ---
>> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> index 3c3d0b99d3e8..0e8d88417251 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> @@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct
>> stmmac_priv *priv)
>> /* Make sure the EPHY is properly reseted, as U-Boot may leave
>> * it at deasserted state, and thus it may fail to reset EMAC.
>> */
>> - reset_control_assert(gmac->rst_ephy);
>> -
>> - ret = reset_control_deassert(gmac->rst_ephy);
>> + ret = reset_control_reset(gmac->rst_ephy);
>> if (ret) {
>> - dev_err(priv->device, "Cannot deassert internal phy\n");
>> + dev_err(priv->device, "Cannot reset internal PHY\n");
>> clk_disable_unprepare(gmac->ephy_clk);
>> return ret;
>> }
>
> I'm assuming you have exclusive access to the phy and this isn't a
> shared line? Just wanting to confirm since the function call has the
> following comment in the header for the documentation.
Yes, this driver has exclusive access:
gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);
And this is a reset line for the Ethernet PHY inside the SoC, that as
far as I can tell is not shared with anything else.
> * Consumers must not use reset_control_(de)assert on shared reset lines when
> * reset_control_reset has been used.
> *
>
> If that is the case it might not hurt to add some documentation to
> your call to reset_control_reset here explaining that it is safe to do
> so since you have exclusive access.
I can expand the comment above this line for v2.
Cheers,
Samuel
Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
On 2/8/21 7:20 AM, Andrew Lunn wrote:
> On Mon, Feb 08, 2021 at 12:31:34AM -0600, Samuel Holland wrote:
>> On 2/8/21 12:28 AM, Samuel Holland wrote:
>>> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
>>> pointers to optional clocks were replaced by NULL to simplify the resume
>>> callback implementation. However, that commit missed that the IS_ERR
>>> check in mv64xxx_of_config should be replaced with a NULL check. As a
>>> result, the check always passes, even for an invalid device tree.
>>
>> Sorry, please ignore this unrelated patch. I accidentally copied it to
>> the wrong directory before sending this series.
>
> Hi Samuel
>
> This patch looks correct. But i don't see it in i2c/for-next, where as
> e5c02cf54154 is. I just want to make sure it does not get lost...
Thanks for the concern. I had already sent it separately[0], to the
appropriate maintainers, so this submission was a duplicate.
Cheers,
Samuel
[0]:
https://lore.kernel.org/lkml/20210208061922.10073-1-sam...@sholland.org/
>Andrew
>
Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
On 2/8/21 12:28 AM, Samuel Holland wrote:
> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> pointers to optional clocks were replaced by NULL to simplify the resume
> callback implementation. However, that commit missed that the IS_ERR
> check in mv64xxx_of_config should be replaced with a NULL check. As a
> result, the check always passes, even for an invalid device tree.
Sorry, please ignore this unrelated patch. I accidentally copied it to
the wrong directory before sending this series.
Samuel
[PATCH net-next RESEND 5/5] net: stmmac: dwmac-sun8i: Add a shutdown callback
The Ethernet MAC and PHY are usually major consumers of power on boards
which may not be able to fully power off (those with no PMIC). Powering
down the MAC and internal PHY saves power while these boards are "off".
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4638d4203af5..926e8d5e8963 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1282,6 +1282,15 @@ static int sun8i_dwmac_remove(struct platform_device
*pdev)
return 0;
}
+static void sun8i_dwmac_shutdown(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+ sun8i_dwmac_exit(pdev, gmac);
+}
+
static const struct of_device_id sun8i_dwmac_match[] = {
{ .compatible = "allwinner,sun8i-h3-emac",
.data = &emac_variant_h3 },
@@ -1302,6 +1311,7 @@ MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
static struct platform_driver sun8i_dwmac_driver = {
.probe = sun8i_dwmac_probe,
.remove = sun8i_dwmac_remove,
+ .shutdown = sun8i_dwmac_shutdown,
.driver = {
.name = "dwmac-sun8i",
.pm = &stmmac_pltfr_pm_ops,
--
2.26.2
[PATCH net-next RESEND 4/5] net: stmmac: dwmac-sun8i: Minor probe function cleanup
Adjust the spacing and use an explicit "return 0" in the success path to make the function easier to parse. Reviewed-by: Chen-Yu Tsai Signed-off-by: Samuel Holland --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 0e8d88417251..4638d4203af5 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -1227,6 +1227,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) ndev = dev_get_drvdata(&pdev->dev); priv = netdev_priv(ndev); + /* The mux must be registered after parent MDIO * so after stmmac_dvr_probe() */ @@ -1245,7 +1246,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) goto dwmac_remove; } - return ret; + return 0; + dwmac_mux: reset_control_put(gmac->rst_ephy); clk_put(gmac->ephy_clk); -- 2.26.2
[PATCH net-next RESEND 0/5] dwmac-sun8i cleanup and shutdown hook
These patches clean up some things I noticed while fixing suspend/resume behavior. The first four are minor code improvements. The last one adds a shutdown hook to minimize power consumption on boards without a PMIC. Now that the fixes series is merged, I'm resending this series rebased on top of net-next and with Chen-Yu's Reviewed-by tags. Samuel Holland (5): net: stmmac: dwmac-sun8i: Return void from PHY unpower net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check net: stmmac: dwmac-sun8i: Use reset_control_reset net: stmmac: dwmac-sun8i: Minor probe function cleanup net: stmmac: dwmac-sun8i: Add a shutdown callback .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 31 --- 1 file changed, 19 insertions(+), 12 deletions(-) -- 2.26.2
[PATCH net-next RESEND 2/5] net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
sun8i_dwmac_unpower_internal_phy already checks if the PHY is powered,
so there is no need to do it again here.
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 8e505019adf8..3c3d0b99d3e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1018,10 +1018,8 @@ static void sun8i_dwmac_exit(struct platform_device
*pdev, void *priv)
{
struct sunxi_priv_data *gmac = priv;
- if (gmac->variant->soc_has_internal_phy) {
- if (gmac->internal_phy_powered)
- sun8i_dwmac_unpower_internal_phy(gmac);
- }
+ if (gmac->variant->soc_has_internal_phy)
+ sun8i_dwmac_unpower_internal_phy(gmac);
clk_disable_unprepare(gmac->tx_clk);
--
2.26.2
[PATCH net-next RESEND 1/5] net: stmmac: dwmac-sun8i: Return void from PHY unpower
This is a deinitialization function that always returned zero, and that
return value was always ignored. Have it return void instead.
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a5e0eff4a387..8e505019adf8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -820,15 +820,14 @@ static int sun8i_dwmac_power_internal_phy(struct
stmmac_priv *priv)
return 0;
}
-static int sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
+static void sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
{
if (!gmac->internal_phy_powered)
- return 0;
+ return;
clk_disable_unprepare(gmac->ephy_clk);
reset_control_assert(gmac->rst_ephy);
gmac->internal_phy_powered = false;
- return 0;
}
/* MDIO multiplexing switch function
--
2.26.2
[PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
Use the appropriate function instead of reimplementing it,
and update the error message to match the code.
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 3c3d0b99d3e8..0e8d88417251 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct
stmmac_priv *priv)
/* Make sure the EPHY is properly reseted, as U-Boot may leave
* it at deasserted state, and thus it may fail to reset EMAC.
*/
- reset_control_assert(gmac->rst_ephy);
-
- ret = reset_control_deassert(gmac->rst_ephy);
+ ret = reset_control_reset(gmac->rst_ephy);
if (ret) {
- dev_err(priv->device, "Cannot deassert internal phy\n");
+ dev_err(priv->device, "Cannot reset internal PHY\n");
clk_disable_unprepare(gmac->ephy_clk);
return ret;
}
--
2.26.2
[PATCH] i2c: mv64xxx: Fix check for missing clock
In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
pointers to optional clocks were replaced by NULL to simplify the resume
callback implementation. However, that commit missed that the IS_ERR
check in mv64xxx_of_config should be replaced with a NULL check. As a
result, the check always passes, even for an invalid device tree.
Fixes: e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support")
Reported-by: Dan Carpenter
Signed-off-by: Samuel Holland
---
drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b03c344323d1..c590d36b5fd1 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -813,7 +813,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
* need to know tclk in order to calculate bus clock
* factors.
*/
- if (IS_ERR(drv_data->clk)) {
+ if (!drv_data->clk) {
rc = -ENODEV;
goto out;
}
--
2.26.2
[PATCH net-next 1/5] net: stmmac: dwmac-sun8i: Return void from PHY unpower
This is a deinitialization function that always returned zero, and that
return value was always ignored. Have it return void instead.
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a5e0eff4a387..8e505019adf8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -820,15 +820,14 @@ static int sun8i_dwmac_power_internal_phy(struct
stmmac_priv *priv)
return 0;
}
-static int sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
+static void sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
{
if (!gmac->internal_phy_powered)
- return 0;
+ return;
clk_disable_unprepare(gmac->ephy_clk);
reset_control_assert(gmac->rst_ephy);
gmac->internal_phy_powered = false;
- return 0;
}
/* MDIO multiplexing switch function
--
2.26.2
[PATCH net-next 5/5] net: stmmac: dwmac-sun8i: Add a shutdown callback
The Ethernet MAC and PHY are usually major consumers of power on boards
which may not be able to fully power off (that have no PMIC). Powering
down the MAC and internal PHY saves power while these boards are "off".
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4638d4203af5..926e8d5e8963 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1282,6 +1282,15 @@ static int sun8i_dwmac_remove(struct platform_device
*pdev)
return 0;
}
+static void sun8i_dwmac_shutdown(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+ sun8i_dwmac_exit(pdev, gmac);
+}
+
static const struct of_device_id sun8i_dwmac_match[] = {
{ .compatible = "allwinner,sun8i-h3-emac",
.data = &emac_variant_h3 },
@@ -1302,6 +1311,7 @@ MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
static struct platform_driver sun8i_dwmac_driver = {
.probe = sun8i_dwmac_probe,
.remove = sun8i_dwmac_remove,
+ .shutdown = sun8i_dwmac_shutdown,
.driver = {
.name = "dwmac-sun8i",
.pm = &stmmac_pltfr_pm_ops,
--
2.26.2
[PATCH net-next 0/5] dwmac-sun8i cleanup and shutdown hook
These patches clean up some things I noticed while fixing suspend/resume behavior. The first four are minor code improvements. The last one adds a shutdown hook to minimize power consumption on boards without a PMIC. Samuel Holland (5): net: stmmac: dwmac-sun8i: Return void from PHY unpower net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check net: stmmac: dwmac-sun8i: Use reset_control_reset net: stmmac: dwmac-sun8i: Minor probe function cleanup net: stmmac: dwmac-sun8i: Add a shutdown callback .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 31 --- 1 file changed, 19 insertions(+), 12 deletions(-) -- 2.26.2
[PATCH net-next 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
Use the appropriate function instead of reimplementing it,
and update the error message to match the code.
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 3c3d0b99d3e8..0e8d88417251 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct
stmmac_priv *priv)
/* Make sure the EPHY is properly reseted, as U-Boot may leave
* it at deasserted state, and thus it may fail to reset EMAC.
*/
- reset_control_assert(gmac->rst_ephy);
-
- ret = reset_control_deassert(gmac->rst_ephy);
+ ret = reset_control_reset(gmac->rst_ephy);
if (ret) {
- dev_err(priv->device, "Cannot deassert internal phy\n");
+ dev_err(priv->device, "Cannot reset internal PHY\n");
clk_disable_unprepare(gmac->ephy_clk);
return ret;
}
--
2.26.2
[PATCH net-next 4/5] net: stmmac: dwmac-sun8i: Minor probe function cleanup
Adjust the spacing and use an explicit "return 0" in the success path to make the function easier to parse. Signed-off-by: Samuel Holland --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 0e8d88417251..4638d4203af5 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -1227,6 +1227,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) ndev = dev_get_drvdata(&pdev->dev); priv = netdev_priv(ndev); + /* The mux must be registered after parent MDIO * so after stmmac_dvr_probe() */ @@ -1245,7 +1246,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) goto dwmac_remove; } - return ret; + return 0; + dwmac_mux: reset_control_put(gmac->rst_ephy); clk_put(gmac->ephy_clk); -- 2.26.2
[PATCH net-next 2/5] net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
sun8i_dwmac_unpower_internal_phy already checks if the PHY is powered,
so there is no need to do it again here.
Signed-off-by: Samuel Holland
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 8e505019adf8..3c3d0b99d3e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1018,10 +1018,8 @@ static void sun8i_dwmac_exit(struct platform_device
*pdev, void *priv)
{
struct sunxi_priv_data *gmac = priv;
- if (gmac->variant->soc_has_internal_phy) {
- if (gmac->internal_phy_powered)
- sun8i_dwmac_unpower_internal_phy(gmac);
- }
+ if (gmac->variant->soc_has_internal_phy)
+ sun8i_dwmac_unpower_internal_phy(gmac);
clk_disable_unprepare(gmac->tx_clk);
--
2.26.2
[PATCH net 3/4] net: stmmac: dwmac-sun8i: Balance internal PHY power
sun8i_dwmac_exit calls sun8i_dwmac_unpower_internal_phy, but
sun8i_dwmac_init did not call sun8i_dwmac_power_internal_phy. This
caused PHY power to remain off after a suspend/resume cycle. Fix this by
recording if PHY power should be restored, and if so, restoring it.
Fixes: 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external
MDIOs")
Signed-off-by: Samuel Holland
---
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 31 ++-
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a05dee5d4584..e2c25c1c702a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -64,6 +64,7 @@ struct emac_variant {
* @variant: reference to the current board variant
* @regmap:regmap for using the syscon
* @internal_phy_powered: Does the internal PHY is enabled
+ * @use_internal_phy: Is the internal PHY selected for use
* @mux_handle:Internal pointer used by mdio-mux lib
*/
struct sunxi_priv_data {
@@ -74,6 +75,7 @@ struct sunxi_priv_data {
const struct emac_variant *variant;
struct regmap_field *regmap_field;
bool internal_phy_powered;
+ bool use_internal_phy;
void *mux_handle;
};
@@ -539,8 +541,11 @@ static const struct stmmac_dma_ops sun8i_dwmac_dma_ops = {
.dma_interrupt = sun8i_dwmac_dma_interrupt,
};
+static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv);
+
static int sun8i_dwmac_init(struct platform_device *pdev, void *priv)
{
+ struct net_device *ndev = platform_get_drvdata(pdev);
struct sunxi_priv_data *gmac = priv;
int ret;
@@ -554,13 +559,25 @@ static int sun8i_dwmac_init(struct platform_device *pdev,
void *priv)
ret = clk_prepare_enable(gmac->tx_clk);
if (ret) {
- if (gmac->regulator)
- regulator_disable(gmac->regulator);
dev_err(&pdev->dev, "Could not enable AHB clock\n");
- return ret;
+ goto err_disable_regulator;
+ }
+
+ if (gmac->use_internal_phy) {
+ ret = sun8i_dwmac_power_internal_phy(netdev_priv(ndev));
+ if (ret)
+ goto err_disable_clk;
}
return 0;
+
+err_disable_clk:
+ clk_disable_unprepare(gmac->tx_clk);
+err_disable_regulator:
+ if (gmac->regulator)
+ regulator_disable(gmac->regulator);
+
+ return ret;
}
static void sun8i_dwmac_core_init(struct mac_device_info *hw,
@@ -831,7 +848,6 @@ static int mdio_mux_syscon_switch_fn(int current_child, int
desired_child,
struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
u32 reg, val;
int ret = 0;
- bool need_power_ephy = false;
if (current_child ^ desired_child) {
regmap_field_read(gmac->regmap_field, ®);
@@ -839,13 +855,12 @@ static int mdio_mux_syscon_switch_fn(int current_child,
int desired_child,
case DWMAC_SUN8I_MDIO_MUX_INTERNAL_ID:
dev_info(priv->device, "Switch mux to internal PHY");
val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
-
- need_power_ephy = true;
+ gmac->use_internal_phy = true;
break;
case DWMAC_SUN8I_MDIO_MUX_EXTERNAL_ID:
dev_info(priv->device, "Switch mux to external PHY");
val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
- need_power_ephy = false;
+ gmac->use_internal_phy = false;
break;
default:
dev_err(priv->device, "Invalid child ID %x\n",
@@ -853,7 +868,7 @@ static int mdio_mux_syscon_switch_fn(int current_child, int
desired_child,
return -EINVAL;
}
regmap_field_write(gmac->regmap_field, val);
- if (need_power_ephy) {
+ if (gmac->use_internal_phy) {
ret = sun8i_dwmac_power_internal_phy(priv);
if (ret)
return ret;
--
2.26.2
[PATCH net 1/4] net: stmmac: dwmac-sun8i: Fix probe error handling
stmmac_pltfr_remove does three things in one function, making it
inapproprate for unwinding the steps in the probe function. Currently,
a failure before the call to stmmac_dvr_probe would leak OF node
references due to missing a call to stmmac_remove_config_dt. And an
error in stmmac_dvr_probe would cause the driver to attempt to remove a
netdevice that was never added. Fix these by reordering the init and
splitting out the error handling steps.
Fixes: 9f93ac8d4085 ("net-next: stmmac: Add dwmac-sun8i")
Fixes: 40a1dcee2d18 ("net: ethernet: dwmac-sun8i: Use the correct function in
exit path")
Signed-off-by: Samuel Holland
---
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 25 +++
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 58e0511badba..b20f261fce5b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1134,10 +1134,6 @@ static int sun8i_dwmac_probe(struct platform_device
*pdev)
if (ret)
return ret;
- plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
- if (IS_ERR(plat_dat))
- return PTR_ERR(plat_dat);
-
gmac = devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL);
if (!gmac)
return -ENOMEM;
@@ -1201,11 +1197,15 @@ static int sun8i_dwmac_probe(struct platform_device
*pdev)
ret = of_get_phy_mode(dev->of_node, &interface);
if (ret)
return -EINVAL;
- plat_dat->interface = interface;
+
+ plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return PTR_ERR(plat_dat);
/* platform data specifying hardware features and callbacks.
* hardware features were copied from Allwinner drivers.
*/
+ plat_dat->interface = interface;
plat_dat->rx_coe = STMMAC_RX_COE_TYPE2;
plat_dat->tx_coe = 1;
plat_dat->has_sun8i = true;
@@ -1216,7 +1216,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
ret = sun8i_dwmac_init(pdev, plat_dat->bsp_priv);
if (ret)
- return ret;
+ goto dwmac_deconfig;
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
if (ret)
@@ -1230,7 +1230,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
if (gmac->variant->soc_has_internal_phy) {
ret = get_ephy_nodes(priv);
if (ret)
- goto dwmac_exit;
+ goto dwmac_remove;
ret = sun8i_dwmac_register_mdio_mux(priv);
if (ret) {
dev_err(&pdev->dev, "Failed to register mux\n");
@@ -1239,15 +1239,20 @@ static int sun8i_dwmac_probe(struct platform_device
*pdev)
} else {
ret = sun8i_dwmac_reset(priv);
if (ret)
- goto dwmac_exit;
+ goto dwmac_remove;
}
return ret;
dwmac_mux:
sun8i_dwmac_unset_syscon(gmac);
+dwmac_remove:
+ stmmac_dvr_remove(&pdev->dev);
dwmac_exit:
- stmmac_pltfr_remove(pdev);
-return ret;
+ sun8i_dwmac_exit(pdev, gmac);
+dwmac_deconfig:
+ stmmac_remove_config_dt(pdev, plat_dat);
+
+ return ret;
}
static const struct of_device_id sun8i_dwmac_match[] = {
--
2.26.2
[PATCH net 0/4] Fixes for dwmac-sun8i suspend/resume
This series fixes issues preventing dwmac-sun8i from working after a suspend/resume cycle. Those issues include the PHY being left powered off, the MAC syscon configuration being reset, and the reference to the reset controller being improperly dropped. They also fix related issues in probe error handling and driver removal. Samuel Holland (4): net: stmmac: dwmac-sun8i: Fix probe error handling net: stmmac: dwmac-sun8i: Balance internal PHY resource references net: stmmac: dwmac-sun8i: Balance internal PHY power net: stmmac: dwmac-sun8i: Balance syscon (de)initialization .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 129 +++--- 1 file changed, 82 insertions(+), 47 deletions(-) -- 2.26.2
[PATCH net 4/4] net: stmmac: dwmac-sun8i: Balance syscon (de)initialization
Previously, sun8i_dwmac_set_syscon was called from a chain of functions
in several different files:
sun8i_dwmac_probe
stmmac_dvr_probe
stmmac_hw_init
stmmac_hwif_init
sun8i_dwmac_setup
sun8i_dwmac_set_syscon
which made the lifetime of the syscon values hard to reason about. Part
of the problem is that there is no similar platform driver callback from
stmmac_dvr_remove. As a result, the driver unset the syscon value in
sun8i_dwmac_exit, but this leaves it uninitialized after a suspend/
resume cycle. It was also unset a second time (outside sun8i_dwmac_exit)
in the probe error path.
Move the init to the earliest available place in sun8i_dwmac_probe
(after stmmac_probe_config_dt, which initializes plat_dat), and the
deinit to the corresponding position in the cleanup order.
Since priv is not filled in until stmmac_dvr_probe, this requires
changing the sun8i_dwmac_set_syscon parameters to priv's two relevant
members.
Fixes: 9f93ac8d4085 ("net-next: stmmac: Add dwmac-sun8i")
Fixes: 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external
MDIOs")
Signed-off-by: Samuel Holland
---
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 +--
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index e2c25c1c702a..a5e0eff4a387 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -898,22 +898,23 @@ static int sun8i_dwmac_register_mdio_mux(struct
stmmac_priv *priv)
return ret;
}
-static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
+static int sun8i_dwmac_set_syscon(struct device *dev,
+ struct plat_stmmacenet_data *plat)
{
- struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
- struct device_node *node = priv->device->of_node;
+ struct sunxi_priv_data *gmac = plat->bsp_priv;
+ struct device_node *node = dev->of_node;
int ret;
u32 reg, val;
ret = regmap_field_read(gmac->regmap_field, &val);
if (ret) {
- dev_err(priv->device, "Fail to read from regmap field.\n");
+ dev_err(dev, "Fail to read from regmap field.\n");
return ret;
}
reg = gmac->variant->default_syscon_value;
if (reg != val)
- dev_warn(priv->device,
+ dev_warn(dev,
"Current syscon value is not the default %x (expect
%x)\n",
val, reg);
@@ -926,9 +927,9 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
/* Force EPHY xtal frequency to 24MHz. */
reg |= H3_EPHY_CLK_SEL;
- ret = of_mdio_parse_addr(priv->device, priv->plat->phy_node);
+ ret = of_mdio_parse_addr(dev, plat->phy_node);
if (ret < 0) {
- dev_err(priv->device, "Could not parse MDIO addr\n");
+ dev_err(dev, "Could not parse MDIO addr\n");
return ret;
}
/* of_mdio_parse_addr returns a valid (0 ~ 31) PHY
@@ -944,17 +945,17 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv
*priv)
if (!of_property_read_u32(node, "allwinner,tx-delay-ps", &val)) {
if (val % 100) {
- dev_err(priv->device, "tx-delay must be a multiple of
100\n");
+ dev_err(dev, "tx-delay must be a multiple of 100\n");
return -EINVAL;
}
val /= 100;
- dev_dbg(priv->device, "set tx-delay to %x\n", val);
+ dev_dbg(dev, "set tx-delay to %x\n", val);
if (val <= gmac->variant->tx_delay_max) {
reg &= ~(gmac->variant->tx_delay_max <<
SYSCON_ETXDC_SHIFT);
reg |= (val << SYSCON_ETXDC_SHIFT);
} else {
- dev_err(priv->device, "Invalid TX clock delay: %d\n",
+ dev_err(dev, "Invalid TX clock delay: %d\n",
val);
return -EINVAL;
}
@@ -962,17 +963,17 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv
*priv)
if (!of_property_read_u32(node, "allwinner,rx-delay-ps", &val)) {
if (val % 100) {
- dev_err(priv->device, "rx-delay must be a multiple of
100\n");
+ dev_err(dev, "rx-delay must be a multiple of
[PATCH net 2/4] net: stmmac: dwmac-sun8i: Balance internal PHY resource references
While stmmac_pltfr_remove calls sun8i_dwmac_exit, the sun8i_dwmac_init
and sun8i_dwmac_exit functions are also called by the stmmac_platform
suspend/resume callbacks. They may be called many times during the
device's lifetime and should not release resources used by the driver.
Furthermore, there was no error handling in case registering the MDIO
mux failed during probe, and the EPHY clock was never released at all.
Fix all of these issues by moving the deinitialization code to a driver
removal callback. Also ensure the EPHY is powered down before removal.
Fixes: 634db83b8265 ("net: stmmac: dwmac-sun8i: Handle integrated/external
MDIOs")
Signed-off-by: Samuel Holland
---
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 27 ++-
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index b20f261fce5b..a05dee5d4584 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1004,17 +1004,12 @@ static void sun8i_dwmac_exit(struct platform_device
*pdev, void *priv)
struct sunxi_priv_data *gmac = priv;
if (gmac->variant->soc_has_internal_phy) {
- /* sun8i_dwmac_exit could be called with mdiomux uninit */
- if (gmac->mux_handle)
- mdio_mux_uninit(gmac->mux_handle);
if (gmac->internal_phy_powered)
sun8i_dwmac_unpower_internal_phy(gmac);
}
sun8i_dwmac_unset_syscon(gmac);
- reset_control_put(gmac->rst_ephy);
-
clk_disable_unprepare(gmac->tx_clk);
if (gmac->regulator)
@@ -1244,6 +1239,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
return ret;
dwmac_mux:
+ reset_control_put(gmac->rst_ephy);
+ clk_put(gmac->ephy_clk);
sun8i_dwmac_unset_syscon(gmac);
dwmac_remove:
stmmac_dvr_remove(&pdev->dev);
@@ -1255,6 +1252,24 @@ static int sun8i_dwmac_probe(struct platform_device
*pdev)
return ret;
}
+static int sun8i_dwmac_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+ struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+ if (gmac->variant->soc_has_internal_phy) {
+ mdio_mux_uninit(gmac->mux_handle);
+ sun8i_dwmac_unpower_internal_phy(gmac);
+ reset_control_put(gmac->rst_ephy);
+ clk_put(gmac->ephy_clk);
+ }
+
+ stmmac_pltfr_remove(pdev);
+
+ return 0;
+}
+
static const struct of_device_id sun8i_dwmac_match[] = {
{ .compatible = "allwinner,sun8i-h3-emac",
.data = &emac_variant_h3 },
@@ -1274,7 +1289,7 @@ MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
static struct platform_driver sun8i_dwmac_driver = {
.probe = sun8i_dwmac_probe,
- .remove = stmmac_pltfr_remove,
+ .remove = sun8i_dwmac_remove,
.driver = {
.name = "dwmac-sun8i",
.pm = &stmmac_pltfr_pm_ops,
--
2.26.2
Re: [PATCH] net/ncsi: Use real net-device for response handler
On Tue, 2020-12-22 at 06:13 +, Joel Stanley wrote:
> On Sun, 20 Dec 2020 at 12:40, John Wang <
> wangzhiqiang...@bytedance.com> wrote:
> >
> > When aggregating ncsi interfaces and dedicated interfaces to bond
> > interfaces, the ncsi response handler will use the wrong net device
> > to
> > find ncsi_dev, so that the ncsi interface will not work properly.
> > Here, we use the net device registered to packet_type to fix it.
> >
> > Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
> > Signed-off-by: John Wang
>
> Can you show me how to reproduce this?
>
> I don't know the ncsi or net code well enough to know if this is the
> correct fix. If you are confident it is correct then I have no
> objections.
This looks like it is probably right; pt->dev will be the original
device from ncsi_register_dev(), if a response comes in to
ncsi_rcv_rsp() associated with a different device then the driver will
fail to find the correct ncsi_dev_priv. An example of the broken case
would be good to see though.
Cheers,
Sam
>
> Cheers,
>
> Joel
>
> > ---
> > net/ncsi/ncsi-rsp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> > index a94bb59793f0..60ae32682904 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -1120,7 +1120,7 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct
> > net_device *dev,
> > int payload, i, ret;
> >
> > /* Find the NCSI device */
> > - nd = ncsi_find_dev(dev);
> > + nd = ncsi_find_dev(pt->dev);
> > ndp = nd ? TO_NCSI_DEV_PRIV(nd) : NULL;
> > if (!ndp)
> > return -ENODEV;
> > --
> > 2.25.1
> >
答复: [PATCH -next] net/mlx5_core: remove unused including
ok, I will add the Fixes line and send the v2 soon.
-邮件原件-
发件人: Leon Romanovsky [mailto:l...@kernel.org]
发送时间: 2020年12月9日 14:21
收件人: Jakub Kicinski
抄送: Zouwei (Samuel) ; sae...@nvidia.com;
da...@davemloft.net; netdev@vger.kernel.org; linux-r...@vger.kernel.org;
linux-ker...@vger.kernel.org
主题: Re: [PATCH -next] net/mlx5_core: remove unused including
On Tue, Dec 08, 2020 at 11:22:26AM -0800, Jakub Kicinski wrote:
> On Mon, 7 Dec 2020 20:14:00 +0800 Zou Wei wrote:
> > Remove including that don't need it.
> >
> > Signed-off-by: Zou Wei
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> > index 989c70c..82ecc161 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> > @@ -30,7 +30,6 @@
> > * SOFTWARE.
> > */
> >
> > -#include
> > #include
> > #include
> > #include
Jakub,
You probably doesn't have latest net-next.
In the commit 17a7612b99e6 ("net/mlx5_core: Clean driver version and name"), I
removed "strlcpy(drvinfo->version, UTS_RELEASE, sizeof(drvinfo->version));"
line.
The patch is ok, but should have Fixes line.
Fixes: 17a7612b99e6 ("net/mlx5_core: Clean driver version and name")
Thanks
>
>
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c: In function
> ‘mlx5e_rep_get_drvinfo’:
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:66:28: error: ‘UTS_RELEASE’
> undeclared (first use in this function); did you mean ‘CSS_RELEASED’?
>66 | strlcpy(drvinfo->version, UTS_RELEASE, sizeof(drvinfo->version));
> |^~~
> |CSS_RELEASED
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:66:28: note: each
> undeclared identifier is reported only once for each function it
> appears in
> make[6]: *** [drivers/net/ethernet/mellanox/mlx5/core/en_rep.o] Error
> 1
> make[5]: *** [drivers/net/ethernet/mellanox/mlx5/core] Error 2
> make[4]: *** [drivers/net/ethernet/mellanox] Error 2
> make[3]: *** [drivers/net/ethernet] Error 2
> make[2]: *** [drivers/net] Error 2
> make[2]: *** Waiting for unfinished jobs
> make[1]: *** [drivers] Error 2
> make: *** [__sub-make] Error 2
Re: [PATCH v2] net/ncsi: Fix netlink registration
On Thu, 2020-11-12 at 16:42 +1030, Joel Stanley wrote:
> If a user unbinds and re-binds a NC-SI aware driver the kernel will
> attempt to register the netlink interface at runtime. The structure
> is
> marked __ro_after_init so registration fails spectacularly at this
> point.
Reviewed-by: Samuel Mendoza-Jonas
>
> # echo 1e66.ethernet >
> /sys/bus/platform/drivers/ftgmac100/unbind
> # echo 1e66.ethernet > /sys/bus/platform/drivers/ftgmac100/bind
> ftgmac100 1e66.ethernet: Read MAC address 52:54:00:12:34:56
> from chip
> ftgmac100 1e66.ethernet: Using NCSI interface
> 8<--- cut here ---
> Unable to handle kernel paging request at virtual address 80a8f858
> pgd = 8c768dd6
> [80a8f858] *pgd=80a0841e(bad)
> Internal error: Oops: 80d [#1] SMP ARM
> CPU: 0 PID: 116 Comm: sh Not tainted 5.10.0-rc3-next-2020-
> 3-gdd25b227ec1e #51
> Hardware name: Generic DT based system
> PC is at genl_register_family+0x1f8/0x6d4
> LR is at 0xff26
> pc : [<8073f930>] lr : [] psr: 2153
> sp : 8553bc80 ip : 81406244 fp : 8553bd04
> r10: 8085d12c r9 : 80a8f73c r8 : 85739000
> r7 : 0017 r6 : 80a8f860 r5 : 80c8ab98 r4 : 80a8f858
> r3 : r2 : r1 : 81406130 r0 : 0017
> Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment none
> Control: 00c5387d Table: 85524008 DAC: 0051
> Process sh (pid: 116, stack limit = 0x1f1988d6)
> ...
> Backtrace:
> [<8073f738>] (genl_register_family) from [<80860ac0>]
> (ncsi_init_netlink+0x20/0x48)
> r10:8085d12c r9:80c8fb0c r8:85739000 r7: r6:81218000
> r5:85739000
> r4:8121c000
> [<80860aa0>] (ncsi_init_netlink) from [<8085d740>]
> (ncsi_register_dev+0x1b0/0x210)
> r5:8121c400 r4:8121c000
> [<8085d590>] (ncsi_register_dev) from [<805a8060>]
> (ftgmac100_probe+0x6e0/0x778)
> r10:0004 r9:80950228 r8:8115bc10 r7:8115ab00 r6:9eae2c24
> r5:813b6f88
> r4:85739000
> [<805a7980>] (ftgmac100_probe) from [<805355ec>]
> (platform_drv_probe+0x58/0xa8)
> r9:80c76bb0 r8: r7:80cd4974 r6:80c76bb0 r5:8115bc10
> r4:
> [<80535594>] (platform_drv_probe) from [<80532d58>]
> (really_probe+0x204/0x514)
> r7:80cd4974 r6: r5:80cd4868 r4:8115bc10
>
> Jakub pointed out that ncsi_register_dev is obviously broken, because
> there is only one family so it would never work if there was more
> than
> one ncsi netdev.
>
> Fix the crash by registering the netlink family once on boot, and
> drop
> the code to unregister it.
>
> Fixes: 955dc68cb9b2 ("net/ncsi: Add generic netlink family")
> Signed-off-by: Joel Stanley
> ---
> v2: Implement Jakub's suggestion
> - drop __ro_after_init change
> - register netlink with subsys_intcall
> - remove unregister function
>
> net/ncsi/ncsi-manage.c | 5 -
> net/ncsi/ncsi-netlink.c | 22 +++---
> net/ncsi/ncsi-netlink.h | 3 ---
> 3 files changed, 3 insertions(+), 27 deletions(-)
>
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index f1be3e3f6425..a9cb355324d1 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1726,9 +1726,6 @@ struct ncsi_dev *ncsi_register_dev(struct
> net_device *dev,
> ndp->ptype.dev = dev;
> dev_add_pack(&ndp->ptype);
>
> - /* Set up generic netlink interface */
> - ncsi_init_netlink(dev);
> -
> pdev = to_platform_device(dev->dev.parent);
> if (pdev) {
> np = pdev->dev.of_node;
> @@ -1892,8 +1889,6 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
> list_del_rcu(&ndp->node);
> spin_unlock_irqrestore(&ncsi_dev_lock, flags);
>
> - ncsi_unregister_netlink(nd->dev);
> -
> kfree(ndp);
> }
> EXPORT_SYMBOL_GPL(ncsi_unregister_dev);
> diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
> index adddc7707aa4..bb5f1650f11c 100644
> --- a/net/ncsi/ncsi-netlink.c
> +++ b/net/ncsi/ncsi-netlink.c
> @@ -766,24 +766,8 @@ static struct genl_family ncsi_genl_family
> __ro_after_init = {
> .n_small_ops = ARRAY_SIZE(ncsi_ops),
> };
>
> -int ncsi_init_netlink(struct net_device *dev)
> +static int __init ncsi_init_netlink(void)
> {
> - int rc;
> -
> - rc = genl_register_family(&ncsi_genl_family);
> - if (rc)
> - netdev_err(dev, "ncsi: failed to register netlink
> family\n");
> -
> - return rc;
> -}
> -
> -int ncsi_unregister_netlink(struct net_device *dev)
> -{
> - int rc;
&
Re: [linux-sunxi] Re: [PATCH] net: phy: realtek: omit setting PHY-side delay when "rgmii" specified
Icenowy, On 10/26/20 7:12 AM, Andrew Lunn wrote: >> By referring to linux/phy.h, NA means not applicable. This surely >> do not apply when RGMII is really in use. > > It means the PHY driver should not touch the mode, something else has > set it up. That could be strapping, the bootloader, ACPI firmware, > whatever. > >> I think no document declares RGMII must have all internal delays >> of the PHY explicitly disabled. It just says RGMII. > > Please take a look at all the other PHY drivers. They should all > disable delays when passed PHY_INTERFACE_MODE_RGMII. Documentation/networking/phy.rst also makes this clear: PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any internal delay by itself, it assumes that either the Ethernet MAC (if capable or the PCB traces) insert the correct 1.5-2ns delay Regards, Samuel
[PATCH -next] net: dsa: sja1105: remove set but not used variable 'prev_time'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/dsa/sja1105/sja1105_vl.c:468:6: warning: variable ‘prev_time’ set
but not used [-Wunused-but-set-variable]
u32 prev_time = 0;
^
Reported-by: Hulk Robot
Signed-off-by: Samuel Zou
---
drivers/net/dsa/sja1105/sja1105_vl.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c
b/drivers/net/dsa/sja1105/sja1105_vl.c
index b52f1af..aa9b0b9 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -465,7 +465,6 @@ sja1105_gating_cfg_time_to_interval(struct
sja1105_gating_config *gating_cfg,
struct sja1105_gate_entry *last_e;
struct sja1105_gate_entry *e;
struct list_head *prev;
- u32 prev_time = 0;
list_for_each_entry(e, &gating_cfg->entries, list) {
struct sja1105_gate_entry *p;
@@ -476,7 +475,6 @@ sja1105_gating_cfg_time_to_interval(struct
sja1105_gating_config *gating_cfg,
continue;
p = list_entry(prev, struct sja1105_gate_entry, list);
- prev_time = e->interval;
p->interval = e->interval - p->interval;
}
last_e = list_last_entry(&gating_cfg->entries,
--
2.6.2
Re: [PATCH -next] iwlwifi: pcie: Use bitwise instead of arithmetic operator for flags
Both of you are right. I neglected, and this patch is wrong. Thanks. On 2020/5/6 23:15, Joe Perches wrote: On Wed, 2020-05-06 at 16:51 +0300, Luciano Coelho wrote: On Tue, 2020-05-05 at 20:19 -0700, Joe Perches wrote: On Wed, 2020-05-06 at 11:07 +0800, Samuel Zou wrote: This silences the following coccinelle warning: "WARNING: sum of probable bitmasks, consider |" I suggest instead ignoring bad and irrelevant warnings. PREFIX_LEN is 32 not 0x20 or BIT(5) PCI_DUMP_SIZE is 352 diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c [] @@ -109,9 +109,9 @@ void iwl_trans_pcie_dump_regs(struct iwl_trans *trans) /* Alloc a max size buffer */ alloc_size = PCI_ERR_ROOT_ERR_SRC + 4 + PREFIX_LEN; - alloc_size = max_t(u32, alloc_size, PCI_DUMP_SIZE + PREFIX_LEN); - alloc_size = max_t(u32, alloc_size, PCI_MEM_DUMP_SIZE + PREFIX_LEN); - alloc_size = max_t(u32, alloc_size, PCI_PARENT_DUMP_SIZE + PREFIX_LEN); + alloc_size = max_t(u32, alloc_size, PCI_DUMP_SIZE | PREFIX_LEN); + alloc_size = max_t(u32, alloc_size, PCI_MEM_DUMP_SIZE | PREFIX_LEN); + alloc_size = max_t(u32, alloc_size, PCI_PARENT_DUMP_SIZE | PREFIX_LEN); buf = kmalloc(alloc_size, GFP_ATOMIC); if (!buf) Yeah, those macros are clearly not bitmasks. I'm dropping this patch. Can the cocci script that generated this warning scripts/coccinelle/misc/orplus.cocci be dropped or improved to validate the likelihood that the defines or constants used are more likely than not are bit values? Maybe these should be defined as hex or BIT or BIT_ULL or GENMASK or the like? Right now it seems it just tests for two constants. .
[PATCH -next] net: ethernet: mediatek: Make mtk_m32 static
Fix the following sparse warning:
drivers/net/ethernet/mediatek/mtk_eth_soc.c:68:5: warning:
symbol 'mtk_m32' was not declared. Should it be static?
Reported-by: Hulk Robot
Signed-off-by: Samuel Zou
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 0904710..2822268 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -65,7 +65,7 @@ u32 mtk_r32(struct mtk_eth *eth, unsigned reg)
return __raw_readl(eth->base + reg);
}
-u32 mtk_m32(struct mtk_eth *eth, u32 mask, u32 set, unsigned reg)
+static u32 mtk_m32(struct mtk_eth *eth, u32 mask, u32 set, unsigned reg)
{
u32 val;
--
2.6.2
[PATCH -next] iwlwifi: pcie: Use bitwise instead of arithmetic operator for flags
This silences the following coccinelle warning: "WARNING: sum of probable bitmasks, consider |" Reported-by: Hulk Robot Signed-off-by: Samuel Zou --- drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c index a0daae0..6d9bf9f 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c @@ -109,9 +109,9 @@ void iwl_trans_pcie_dump_regs(struct iwl_trans *trans) /* Alloc a max size buffer */ alloc_size = PCI_ERR_ROOT_ERR_SRC + 4 + PREFIX_LEN; - alloc_size = max_t(u32, alloc_size, PCI_DUMP_SIZE + PREFIX_LEN); - alloc_size = max_t(u32, alloc_size, PCI_MEM_DUMP_SIZE + PREFIX_LEN); - alloc_size = max_t(u32, alloc_size, PCI_PARENT_DUMP_SIZE + PREFIX_LEN); + alloc_size = max_t(u32, alloc_size, PCI_DUMP_SIZE | PREFIX_LEN); + alloc_size = max_t(u32, alloc_size, PCI_MEM_DUMP_SIZE | PREFIX_LEN); + alloc_size = max_t(u32, alloc_size, PCI_PARENT_DUMP_SIZE | PREFIX_LEN); buf = kmalloc(alloc_size, GFP_ATOMIC); if (!buf) -- 2.6.2
[PATCH -next] net: ethernet: ti: Use PTR_ERR_OR_ZERO() to simplify code
Fixes coccicheck warning:
drivers/net/ethernet/ti/am65-cpts.c:1017:1-3: WARNING: PTR_ERR_OR_ZERO can be
used
Reported-by: Hulk Robot
Signed-off-by: Samuel Zou
---
drivers/net/ethernet/ti/am65-cpts.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ti/am65-cpts.c
b/drivers/net/ethernet/ti/am65-cpts.c
index 370162c..51c94b2 100644
--- a/drivers/net/ethernet/ti/am65-cpts.c
+++ b/drivers/net/ethernet/ti/am65-cpts.c
@@ -1014,10 +1014,7 @@ static int am65_cpts_probe(struct platform_device *pdev)
return PTR_ERR(base);
cpts = am65_cpts_create(dev, base, node);
- if (IS_ERR(cpts))
- return PTR_ERR(cpts);
-
- return 0;
+ return PTR_ERR_OR_ZERO(cpts);
}
static const struct of_device_id am65_cpts_of_match[] = {
--
2.6.2
Re: [PATCH -next v2] hinic: Use ARRAY_SIZE for nic_vf_cmd_msg_handler
On 2020/4/30 2:43, David Miller wrote: From: Zou Wei Date: Wed, 29 Apr 2020 12:17:40 +0800 fix coccinelle warning, use ARRAY_SIZE drivers/net/ethernet/huawei/hinic/hinic_sriov.c:713:43-44: WARNING: Use ARRAY_SIZE -- Please don't put this "---" here. diff --git a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c index b24788e..af70cca 100644 --- a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c +++ b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c @@ -704,17 +704,15 @@ int nic_pf_mbox_handler(void *hwdev, u16 vf_id, u8 cmd, void *buf_in, struct hinic_hwdev *dev = hwdev; struct hinic_func_to_io *nic_io; struct hinic_pfhwdev *pfhwdev; - u32 i, cmd_number; + u32 i; int err = 0; Please preserve the reverse christmas tree ordering of local variables. . Thanks，I will modify and send v3 patch
Re: [PATCH -next] hinic: Use ARRAY_SIZE for nic_vf_cmd_msg_handler
Hi Joe,
Thanks for your comments, I will modify and send the v2
On 2020/4/29 11:23, Joe Perches wrote:
On Wed, 2020-04-29 at 11:15 +0800, Zou Wei wrote:
fix coccinelle warning, use ARRAY_SIZE
drivers/net/ethernet/huawei/hinic/hinic_sriov.c:713:43-44: WARNING: Use
ARRAY_SIZE
Reported-by: Hulk Robot
Signed-off-by: Zou Wei
---
drivers/net/ethernet/huawei/hinic/hinic_sriov.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
index b24788e..ac12725 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
@@ -710,8 +710,7 @@ int nic_pf_mbox_handler(void *hwdev, u16 vf_id, u8 cmd,
void *buf_in,
if (!hwdev)
return -EFAULT;
- cmd_number = sizeof(nic_vf_cmd_msg_handler) /
- sizeof(struct vf_cmd_msg_handle);
+ cmd_number = ARRAY_SIZE(nic_vf_cmd_msg_handler);
pfhwdev = container_of(dev, struct hinic_pfhwdev, hwdev);
nic_io = &dev->func_to_io;
for (i = 0; i < cmd_number; i++) {
Probably better to remove cmd_number altogether
---
drivers/net/ethernet/huawei/hinic/hinic_sriov.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
index b24788..af70cc 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_sriov.c
@@ -704,17 +704,15 @@ int nic_pf_mbox_handler(void *hwdev, u16 vf_id, u8 cmd,
void *buf_in,
struct hinic_hwdev *dev = hwdev;
struct hinic_func_to_io *nic_io;
struct hinic_pfhwdev *pfhwdev;
- u32 i, cmd_number;
+ u32 i;
int err = 0;
if (!hwdev)
return -EFAULT;
- cmd_number = sizeof(nic_vf_cmd_msg_handler) /
- sizeof(struct vf_cmd_msg_handle);
pfhwdev = container_of(dev, struct hinic_pfhwdev, hwdev);
nic_io = &dev->func_to_io;
- for (i = 0; i < cmd_number; i++) {
+ for (i = 0; i < ARRAY_SIZE(nic_vf_cmd_msg_handler); i++) {
vf_msg_handle = &nic_vf_cmd_msg_handler[i];
if (cmd == vf_msg_handle->cmd &&
vf_msg_handle->cmd_msg_handler) {
@@ -725,7 +723,7 @@ int nic_pf_mbox_handler(void *hwdev, u16 vf_id, u8 cmd,
void *buf_in,
break;
}
}
- if (i == cmd_number)
+ if (i == ARRAY_SIZE(nic_vf_cmd_msg_handler))
err = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_L2NIC,
cmd, buf_in, in_size, buf_out,
out_size, HINIC_MGMT_MSG_SYNC);
.
Re: [PATCH] NFC: Orphan the subsystem
Hi Johannes, On Tue, May 14, 2019 at 11:02:31AM +0200, Johannes Berg wrote: > Samuel clearly hasn't been working on this in many years and > patches getting to the wireless list are just being ignored > entirely now. Mark the subsystem as orphan to reflect the > current state and revert back to the netdev list so at least > some fixes can be picked up by Dave. > > Signed-off-by: Johannes Berg I meant to do that for many months now but did not find time to push it to the top of my TODO. Thanks. Acked-by: Samuel Ortiz > --- > MAINTAINERS | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index fb9f9d71f7a2..b2659312e9ed 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11028,10 +11028,8 @@ S: Supported > F: drivers/net/ethernet/qlogic/netxen/ > > NFC SUBSYSTEM > -M: Samuel Ortiz > -L: linux-wirel...@vger.kernel.org > -L: linux-...@lists.01.org (subscribers-only) > -S: Supported > +L: netdev@vger.kernel.org > +S: Orphan > F: net/nfc/ > F: include/net/nfc/ > F: include/uapi/linux/nfc.h > -- > 2.17.2 >
Re: [PATCH net-next v7] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
On Thu, 2018-10-11 at 18:07 +, justin.l...@dell.com wrote: > The new command (NCSI_CMD_SEND_CMD) is added to allow user space application > to send NC-SI command to the network card. > Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and > response. > > The work flow is as below. > > Request: > User space application > -> Netlink interface (msg) > -> new Netlink handler - ncsi_send_cmd_nl() > -> ncsi_xmit_cmd() > > Response: > Response received - ncsi_rcv_rsp() > -> internal response handler - ncsi_rsp_handler_xxx() > -> ncsi_rsp_handler_netlink() > -> ncsi_send_netlink_rsp () > -> Netlink interface (msg) > -> user space application > > Command timeout - ncsi_request_timeout() > -> ncsi_send_netlink_timeout () > -> Netlink interface (msg with zero data length) > -> user space application > > Error: > Error detected > -> ncsi_send_netlink_err () > -> Netlink interface (err msg) > -> user space application > > > Signed-off-by: Justin Lee > Reviewed-by: Samuel Mendoza-Jonas
Re: [RFC PATCH 2/2] net/ncsi: Configure multi-package, multi-channel modes with failover
On Wed, 2018-10-10 at 22:36 +, justin.l...@dell.com wrote:
> Hi Samuel,
>
> I am still testing your change and have some comments below.
>
> Thanks,
> Justin
>
>
> > This patch extends the ncsi-netlink interface with two new commands and
> > three new attributes to configure multiple packages and/or channels at
> > once, and configure specific failover modes.
> >
> > NCSI_CMD_SET_PACKAGE mask and NCSI_CMD_SET_CHANNEL_MASK set a whitelist
> > of packages or channels allowed to be configured with the
> > NCSI_ATTR_PACKAGE_MASK and NCSI_ATTR_CHANNEL_MASK attributes
> > respectively. If one of these whitelists is set only packages or
> > channels matching the whitelist are considered for the channel queue in
> > ncsi_choose_active_channel().
> >
> > These commands may also use the NCSI_ATTR_MULTI_FLAG to signal that
> > multiple packages or channels may be configured simultaneously. NCSI
> > hardware arbitration (HWA) must be available in order to enable
> > multi-package mode. Multi-channel mode is always available.
> >
> > If the NCSI_ATTR_CHANNEL_ID attribute is present in the
> > NCSI_CMD_SET_CHANNEL_MASK command the it sets the preferred channel as
> > with the NCSI_CMD_SET_INTERFACE command. The combination of preferred
> > channel and channel whitelist defines a primary channel and the allowed
> > failover channels.
> > If the NCSI_ATTR_MULTI_FLAG attribute is also present then the preferred
> > channel is configured for Tx/Rx and the other channels are enabled only
> > for Rx.
> >
> > Signed-off-by: Samuel Mendoza-Jonas
> > ---
> > include/uapi/linux/ncsi.h | 16 +++
> > net/ncsi/internal.h | 11 +-
> > net/ncsi/ncsi-aen.c | 2 +-
> > net/ncsi/ncsi-manage.c| 138
> > net/ncsi/ncsi-netlink.c | 217 +-
> > net/ncsi/ncsi-rsp.c | 2 +-
> > 6 files changed, 312 insertions(+), 74 deletions(-)
> >
> > diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> > index 4c292ecbb748..035fba1693f9 100644
> > --- a/include/uapi/linux/ncsi.h
> > +++ b/include/uapi/linux/ncsi.h
> > @@ -23,6 +23,13 @@
> > * optionally the preferred NCSI_ATTR_CHANNEL_ID.
> > * @NCSI_CMD_CLEAR_INTERFACE: clear any preferred package/channel
> > combination.
> > * Requires NCSI_ATTR_IFINDEX.
> > + * @NCSI_CMD_SET_PACKAGE_MASK: set a whitelist of allowed packages.
> > + * @NCSI_CMD_SET_PACKAGE_MASK: set a whitelist of allowed channels.
> > + * Requires NCSI_ATTR_IFINDEX and NCSI_ATTR_PACKAGE_MASK.
> > + * @NCSI_CMD_SET_PACKAGE_MASK: set a whitelist of allowed channels.
> > + * Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID, and
> > + * NCSI_ATTR_CHANNEL_MASK. If NCSI_ATTR_CHANNEL_ID is present it sets
> > + * the primary channel.
> > * @NCSI_CMD_MAX: highest command number
> > */
>
> There are some typo in the description.
> * @NCSI_CMD_SET_PACKAGE_MASK: set a whitelist of allowed packages.
> *Requires NCSI_ATTR_IFINDEX and NCSI_ATTR_PACKAGE_MASK.
> * @NCSI_CMD_SET_CHANNEL_MASK: set a whitelist of allowed channels.
> *Requires NCSI_ATTR_IFINDEX, NCSI_ATTR_PACKAGE_ID, and
> *NCSI_ATTR_CHANNEL_MASK. If NCSI_ATTR_CHANNEL_ID is present it sets
> *the primary channel.
Haha, yes I threw that in at the end, thanks for catching it.
>
> > enum ncsi_nl_commands {
> > @@ -30,6 +37,8 @@ enum ncsi_nl_commands {
> > NCSI_CMD_PKG_INFO,
> > NCSI_CMD_SET_INTERFACE,
> > NCSI_CMD_CLEAR_INTERFACE,
> > + NCSI_CMD_SET_PACKAGE_MASK,
> > + NCSI_CMD_SET_CHANNEL_MASK,
> >
> > __NCSI_CMD_AFTER_LAST,
> > NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> > @@ -43,6 +52,10 @@ enum ncsi_nl_commands {
> > * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> > * @NCSI_ATTR_PACKAGE_ID: package ID
> > * @NCSI_ATTR_CHANNEL_ID: channel ID
> > + * @NCSI_ATTR_MULTI_FLAG: flag to signal that multi-mode should be enabled
> > with
> > + * NCSI_CMD_SET_PACKAGE_MASK or NCSI_CMD_SET_CHANNEL_MASK.
> > + * @NCSI_ATTR_PACKAGE_MASK: 32-bit mask of allowed packages.
> > + * @NCSI_ATTR_CHANNEL_MASK: 32-bit mask of allowed channels.
> > * @NCSI_ATTR_MAX: highest attribute number
> > */
> > enum ncsi_nl_attrs {
> > @@ -51,6 +64,9 @@ enum ncsi_nl_attrs {
> > NCSI_ATTR_PACKAGE_LIST,
> > NCSI_ATTR_PACKAGE_ID,
> > NCSI_ATTR_CHANNEL_ID,
> > + NCSI_ATTR_MULTI_FLAG,
> > + NCSI_ATTR_PACKAGE_MASK,
> > + NCSI_ATTR_CHANNEL_MASK,
>
> Is t
Re: [PATCH net-next v5] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
On Wed, 2018-10-10 at 18:11 +, justin.l...@dell.com wrote:
> +
> + len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> + if (len < sizeof(struct ncsi_pkt_hdr)) {
> + netdev_info(ndp->ndev.dev, "NCSI: no command to send %u\n",
> + package_id);
> + ret = -EINVAL;
> + goto out_netlink;
> + } else {
> + data = (unsigned char *)nla_data(info->attrs[NCSI_ATTR_DATA]);
> + }
I only just noticed this, the call to nla_len() can cause a null-dereference if
the NCSI_ATTR_DATA attribute isn't present; we need to make sure it exists
before accessing it in info->attrs.
eg:
root@ozrom2-bmc:~# ./ncsi-netlink -l 2 -p 0 -c 0 --cmd
[ 81.399837] Unable to handle kernel NULL pointer dereference at virtual
address
[ 81.409092] pgd = ddaa9fa6
[ 81.413084] [] *pgd=9702c831, *pte=, *ppte=
[ 81.420729] Internal error: Oops: 17 [#1] ARM
[ 81.426447] CPU: 0 PID: 1028 Comm: ncsi-netlink Not tainted
4.18.8-sammj-00144-gbc129f31bfa5 #12
...
[ 81.874434] Kernel panic - not syncing: Fatal exception
Cheers,
Sam
Re: [PATCH net-next v3] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
On Mon, 2018-10-08 at 23:13 +, justin.l...@dell.com wrote:
> The new command (NCSI_CMD_SEND_CMD) is added to allow user space
> application to send NC-SI command to the network card.
> Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and
> response.
>
> The work flow is as below.
>
> Request:
> User space application
> -> Netlink interface (msg)
> -> new Netlink handler - ncsi_send_cmd_nl()
> -> ncsi_xmit_cmd()
>
> Response:
> Response received - ncsi_rcv_rsp()
> -> internal response handler - ncsi_rsp_handler_xxx()
> -> ncsi_rsp_handler_netlink()
> -> ncsi_send_netlink_rsp ()
> -> Netlink interface (msg)
> -> user space application
>
> Command timeout - ncsi_request_timeout()
> -> ncsi_send_netlink_timeout ()
> -> Netlink interface (msg with zero data length)
> -> user space application
>
> Error:
> Error detected
> -> ncsi_send_netlink_err ()
> -> Netlink interface (err msg)
> -> user space application
Hi Justin,
I've built and tested this and it works as expected; except for some very
minor comments below:
Reviewed-by: Samuel Mendoza-Jonas
>
> V3: Based on http://patchwork.ozlabs.org/patch/979688/ to remove the
> duplicated code.
> V2: Remove non-related debug message and clean up the code.
It's better to put these change notes under the --- below so they're not
included in the commit message, but thanks for including them!
>
>
> Signed-off-by: Justin Lee
>
>
> ---
> include/uapi/linux/ncsi.h | 3 +
> net/ncsi/internal.h | 10 ++-
> net/ncsi/ncsi-cmd.c | 8 ++
> net/ncsi/ncsi-manage.c| 16
> net/ncsi/ncsi-netlink.c | 204
> ++
> net/ncsi/ncsi-netlink.h | 12 +++
> net/ncsi/ncsi-rsp.c | 67 +--
> 7 files changed, 314 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> index 4c292ec..4992bfc 100644
> --- a/include/uapi/linux/ncsi.h
> +++ b/include/uapi/linux/ncsi.h
> @@ -30,6 +30,7 @@ enum ncsi_nl_commands {
> NCSI_CMD_PKG_INFO,
> NCSI_CMD_SET_INTERFACE,
> NCSI_CMD_CLEAR_INTERFACE,
> + NCSI_CMD_SEND_CMD,
>
> __NCSI_CMD_AFTER_LAST,
> NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> @@ -43,6 +44,7 @@ enum ncsi_nl_commands {
> * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> * @NCSI_ATTR_PACKAGE_ID: package ID
> * @NCSI_ATTR_CHANNEL_ID: channel ID
> + * @NCSI_ATTR_DATA: command payload
> * @NCSI_ATTR_MAX: highest attribute number
> */
> enum ncsi_nl_attrs {
> @@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
> NCSI_ATTR_PACKAGE_LIST,
> NCSI_ATTR_PACKAGE_ID,
> NCSI_ATTR_CHANNEL_ID,
> + NCSI_ATTR_DATA,
>
> __NCSI_ATTR_AFTER_LAST,
> NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 3d0a33b..e9db100 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -175,6 +175,8 @@ struct ncsi_package;
> #define NCSI_RESERVED_CHANNEL0x1f
> #define NCSI_CHANNEL_INDEX(c)((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
> #define NCSI_TO_CHANNEL(p, c)(((p) << NCSI_PACKAGE_SHIFT) | (c))
> +#define NCSI_MAX_PACKAGE 8
> +#define NCSI_MAX_CHANNEL 32
>
> struct ncsi_channel {
> unsigned char id;
> @@ -219,12 +221,17 @@ struct ncsi_request {
> unsigned charid; /* Request ID - 0 to 255 */
> bool used;/* Request that has been assigned */
> unsigned int flags; /* NCSI request property */
> -#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
> struct ncsi_dev_priv *ndp;/* Associated NCSI device */
> struct sk_buff *cmd;/* Associated NCSI command packet */
> struct sk_buff *rsp;/* Associated NCSI response packet */
> struct timer_listtimer; /* Timer on waiting for response */
> bool enabled; /* Time has been enabled or not*/
> +
> + u32 snd_seq; /* netlink sending sequence number */
> + u32 snd_portid; /* netlink portid of sender*/
> + struct nlmsghdr nlhdr; /* netlink message header */
> };
>
> enum {
> @@ -310,6 +317,7 @@ struct ncsi_cmd_arg {
> unsigned int dwords[4];
> };
> unsigned char
Re: [PATCH v3] net/ncsi: Add NCSI OEM command support
On Wed, 2018-10-03 at 16:32 -0700, Vijay Khemka wrote:
> This patch adds OEM commands and response handling. It also defines OEM
> command and response structure as per NCSI specification along with its
> handlers.
>
> ncsi_cmd_handler_oem: This is a generic command request handler for OEM
> commands
> ncsi_rsp_handler_oem: This is a generic response handler for OEM commands
>
> Signed-off-by: Vijay Khemka
Reviewed-by: Samuel Mendoza-Jonas
Technically this patch should also be marked [PATCH net-next] to target
David's next tree.
> ---
> net/ncsi/internal.h | 5 +
> net/ncsi/ncsi-cmd.c | 30 +++---
> net/ncsi/ncsi-pkt.h | 14 ++
> net/ncsi/ncsi-rsp.c | 43 ++-
> 4 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 8055e3965cef..3d0a33b874f5 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -68,6 +68,10 @@ enum {
> NCSI_MODE_MAX
> };
>
> +/* OEM Vendor Manufacture ID */
> +#define NCSI_OEM_MFR_MLX_ID 0x8119
> +#define NCSI_OEM_MFR_BCM_ID 0x113d
> +
> struct ncsi_channel_version {
> u32 version;/* Supported BCD encoded NCSI version */
> u32 alpha2; /* Supported BCD encoded NCSI version */
> @@ -305,6 +309,7 @@ struct ncsi_cmd_arg {
> unsigned short words[8];
> unsigned int dwords[4];
> };
> + unsigned char*data; /* NCSI OEM data */
> };
>
> extern struct list_head ncsi_dev_list;
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 7567ca63aae2..82b7d9201db8 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> return 0;
> }
>
> +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> + struct ncsi_cmd_arg *nca)
> +{
> + struct ncsi_cmd_oem_pkt *cmd;
> + unsigned int len;
> +
> + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> + if (nca->payload < 26)
> + len += 26;
> + else
> + len += nca->payload;
> +
> + cmd = skb_put_zero(skb, len);
> + memcpy(&cmd->mfr_id, nca->data, nca->payload);
> + ncsi_cmd_build_header(&cmd->cmd.common, nca);
> +
> + return 0;
> +}
> +
> static struct ncsi_cmd_handler {
> unsigned char type;
> int payload;
> @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
> { NCSI_PKT_CMD_GNS,0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GPS,0, ncsi_cmd_handler_default },
> - { NCSI_PKT_CMD_OEM,0, NULL },
> + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
> { NCSI_PKT_CMD_PLDM, 0, NULL },
> { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> };
> @@ -316,8 +335,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> return -ENOENT;
> }
>
> - /* Get packet payload length and allocate the request */
> - nca->payload = nch->payload;
> + /* Get packet payload length and allocate the request
> + * It is expected that if length set as negative in
> + * handler structure means caller is initializing it
> + * and setting length in nca before calling xmit function
> + */
> + if (nch->payload >= 0)
> + nca->payload = nch->payload;
> nr = ncsi_alloc_command(nca);
> if (!nr)
> return -ENOMEM;
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 91b4b66438df..0f2087c8d42a 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -151,6 +151,20 @@ struct ncsi_cmd_snfc_pkt {
> unsigned char pad[22];
> };
>
> +/* OEM Request Command as per NCSI Specification */
> +struct ncsi_cmd_oem_pkt {
> + struct ncsi_cmd_pkt_hdr cmd; /* Command header*/
> + __be32 mfr_id; /* Manufacture ID*/
> + unsigned char data[]; /* OEM Payload Data */
> +};
> +
> +/* OEM Response Packet as per NCSI Specification */
> +struct ncsi_rsp_oem_pkt {
> + struct ncsi_rsp_pkt_hdr rsp; /* Command header*/
> + __be32 mfr_id; /* Manufacture ID*/
> + unsigned char data[]; /* Payload data */
> +};
> +
> /* Get Link Status */
> struct ncsi_rsp_gls_pkt {
> struct ncsi_rsp_pkt_hdr rsp;/* R
Re: [PATCH net v2] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
On Fri, 2018-09-28 at 18:15 +, justin.l...@dell.com wrote:
> The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
> to send NC-SI command to the network card.
> Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and
> response.
>
> The work flow is as below.
>
> Request:
> User space application -> Netlink interface (msg)
> -> new Netlink handler -
> ncsi_send_cmd_nl()
> -> ncsi_xmit_cmd()
> Response:
> Response received - ncsi_rcv_rsp() -> internal response handler -
> ncsi_rsp_handler_xxx()
> ->
> ncsi_rsp_handler_netlink()
> ->
> ncsi_send_netlink_rsp ()
> ->
> Netlink interface (msg)
> ->
> user space application
> Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout ()
>
> -> Netlink interface (msg with zero data length)
>
> -> user space application
> Error:
> Error detected -> ncsi_send_netlink_err () -> Netlink interface (err msg)
>
> -> user space application
>
>
> Signed-off-by: Justin Lee
Hi Justin,
This is looking pretty good, combined with Vijay's base patch the two
approaches should fit together nicely (
http://patchwork.ozlabs.org/patch/976510/).
A good merge order would probably be the above patch first, then this
patch and Vijay's further OEM patches based on top of that to reduce
conflicts.
Cheers,
Sam
>
> ---
> include/uapi/linux/ncsi.h | 3 +
> net/ncsi/internal.h | 12 ++-
> net/ncsi/ncsi-cmd.c | 47 ++-
> net/ncsi/ncsi-manage.c| 22 +
> net/ncsi/ncsi-netlink.c | 205
> ++
> net/ncsi/ncsi-netlink.h | 12 +++
> net/ncsi/ncsi-rsp.c | 71 ++--
> 7 files changed, 363 insertions(+), 9 deletions(-)
>
> diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> index 4c292ec..4992bfc 100644
> --- a/include/uapi/linux/ncsi.h
> +++ b/include/uapi/linux/ncsi.h
> @@ -30,6 +30,7 @@ enum ncsi_nl_commands {
> NCSI_CMD_PKG_INFO,
> NCSI_CMD_SET_INTERFACE,
> NCSI_CMD_CLEAR_INTERFACE,
> + NCSI_CMD_SEND_CMD,
>
> __NCSI_CMD_AFTER_LAST,
> NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> @@ -43,6 +44,7 @@ enum ncsi_nl_commands {
> * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> * @NCSI_ATTR_PACKAGE_ID: package ID
> * @NCSI_ATTR_CHANNEL_ID: channel ID
> + * @NCSI_ATTR_DATA: command payload
> * @NCSI_ATTR_MAX: highest attribute number
> */
> enum ncsi_nl_attrs {
> @@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
> NCSI_ATTR_PACKAGE_LIST,
> NCSI_ATTR_PACKAGE_ID,
> NCSI_ATTR_CHANNEL_ID,
> + NCSI_ATTR_DATA,
>
> __NCSI_ATTR_AFTER_LAST,
> NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 8055e39..1a3ef9e 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -171,6 +171,8 @@ struct ncsi_package;
> #define NCSI_RESERVED_CHANNEL0x1f
> #define NCSI_CHANNEL_INDEX(c)((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
> #define NCSI_TO_CHANNEL(p, c)(((p) << NCSI_PACKAGE_SHIFT) | (c))
> +#define NCSI_MAX_PACKAGE 8
> +#define NCSI_MAX_CHANNEL 32
>
> struct ncsi_channel {
> unsigned char id;
> @@ -215,12 +217,17 @@ struct ncsi_request {
> unsigned charid; /* Request ID - 0 to 255 */
> bool used;/* Request that has been assigned */
> unsigned int flags; /* NCSI request property */
> -#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
> struct ncsi_dev_priv *ndp;/* Associated NCSI device */
> struct sk_buff *cmd;/* Associated NCSI command packet */
> struct sk_buff *rsp;/* Associated NCSI response packet */
> struct timer_listtimer; /* Timer on waiting for response */
> bool enabled; /* Time has been enabled or not*/
> +
> + u32 snd_seq; /* netlink sending sequence number */
> + u32 snd_portid; /* netlink portid of sender*/
> + struct nlmsghdr nlhdr; /* netlink message header */
> };
>
> enum {
> @@ -305,6 +312,9 @@ struct ncsi_cmd_arg {
> unsig
Re: [PATCH v2] net/ncsi: Add NCSI OEM command support
On Fri, 2018-09-28 at 18:06 -0700, Vijay Khemka wrote:
> This patch adds OEM commands and response handling. It also defines OEM
> command and response structure as per NCSI specification along with its
> handlers.
>
> ncsi_cmd_handler_oem: This is a generic command request handler for OEM
> commands
> ncsi_rsp_handler_oem: This is a generic response handler for OEM commands
>
> Signed-off-by: Vijay Khemka
Hi Vijay - looks good to me, and should be a good common base for your
and Justin's changes.
Reviewed-by: Samuel Mendoza-Jonas
> ---
> net/ncsi/internal.h | 4
> net/ncsi/ncsi-cmd.c | 31 ---
> net/ncsi/ncsi-pkt.h | 16
> net/ncsi/ncsi-rsp.c | 44 +++-
> 4 files changed, 91 insertions(+), 4 deletions(-)
>
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 8055e3965cef..c16cb7223064 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -68,6 +68,10 @@ enum {
> NCSI_MODE_MAX
> };
>
> +/* OEM Vendor Manufacture ID */
> +#define NCSI_OEM_MFR_MLX_ID 0x8119
> +#define NCSI_OEM_MFR_BCM_ID 0x113d
> +
> struct ncsi_channel_version {
> u32 version;/* Supported BCD encoded NCSI version */
> u32 alpha2; /* Supported BCD encoded NCSI version */
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 7567ca63aae2..2f98533eba46 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -211,6 +211,26 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> return 0;
> }
>
> +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> + struct ncsi_cmd_arg *nca)
> +{
> + struct ncsi_cmd_oem_pkt *cmd;
> + unsigned int len;
> +
> + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> + if (nca->payload < 26)
> + len += 26;
> + else
> + len += nca->payload;
> +
> + cmd = skb_put_zero(skb, len);
> + cmd->mfr_id = nca->dwords[0];
> + memcpy(cmd->data, &nca->dwords[1], nca->payload - 4);
> + ncsi_cmd_build_header(&cmd->cmd.common, nca);
> +
> + return 0;
> +}
> +
> static struct ncsi_cmd_handler {
> unsigned char type;
> int payload;
> @@ -244,7 +264,7 @@ static struct ncsi_cmd_handler {
> { NCSI_PKT_CMD_GNS,0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GPS,0, ncsi_cmd_handler_default },
> - { NCSI_PKT_CMD_OEM,0, NULL },
> + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
> { NCSI_PKT_CMD_PLDM, 0, NULL },
> { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> };
> @@ -316,8 +336,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> return -ENOENT;
> }
>
> - /* Get packet payload length and allocate the request */
> - nca->payload = nch->payload;
> + /* Get packet payload length and allocate the request
> + * It is expected that if length set as negative in
> + * handler structure means caller is initializing it
> + * and setting length in nca before calling xmit function
> + */
> + if (nch->payload >= 0)
> + nca->payload = nch->payload;
> nr = ncsi_alloc_command(nca);
> if (!nr)
> return -ENOMEM;
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 91b4b66438df..1f338386810d 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
> unsigned char pad[22];
> };
>
> +/* OEM Request Command as per NCSI Specification */
> +struct ncsi_cmd_oem_pkt {
> + struct ncsi_cmd_pkt_hdr cmd; /* Command header*/
> + __be32 mfr_id; /* Manufacture ID*/
> + unsigned char data[64];/* OEM Payload Data */
> + __be32 checksum;/* Checksum */
> +};
> +
> +/* OEM Response Packet as per NCSI Specification */
> +struct ncsi_rsp_oem_pkt {
> + struct ncsi_rsp_pkt_hdr rsp; /* Command header*/
> + __be32 mfr_id; /* Manufacture ID*/
> + unsigned char data[64];/* Payload data */
> + __be32 checksum;/* Checksum */
> +};
> +
> /* Get Link Status */
> struct ncsi_rsp_gls_pkt {
> struct ncsi_rsp_pkt_hdr rsp;/* Response header */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
>
Re: [PATCH net] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
On Thu, 2018-09-27 at 21:08 +, justin.l...@dell.com wrote:
> The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
> to send NC-SI command to the network card.
> Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and
> response.
>
> The work flow is as below.
>
> Request:
> User space application -> Netlink interface (msg)
> -> new Netlink handler -
> ncsi_send_cmd_nl()
> -> ncsi_xmit_cmd()
> Response:
> Response received - ncsi_rcv_rsp() -> internal response handler -
> ncsi_rsp_handler_xxx()
> ->
> ncsi_rsp_handler_netlink()
> ->
> ncsi_send_netlink_rsp ()
> ->
> Netlink interface (msg)
> ->
> user space application
> Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout ()
>
> -> Netlink interface (msg with zero data length)
>
> -> user space application
> Error:
> Error detected -> ncsi_send_netlink_err () -> Netlink interface (err msg)
>
> -> user space application
>
>
> Signed-off-by: Justin Lee
>
Hi Justin,
Thanks for posting this on the list! The overall design looks good and so
far looks like it should fit relatively well with the other OEM command
patch. I'll try and run some OEM commands against my machine.
Some comments below:
>
> ---
> include/uapi/linux/ncsi.h | 3 +
> net/ncsi/internal.h | 12 ++-
> net/ncsi/ncsi-aen.c | 10 ++-
> net/ncsi/ncsi-cmd.c | 106
> net/ncsi/ncsi-manage.c| 74 ++---
> net/ncsi/ncsi-netlink.c | 199
> +-
> net/ncsi/ncsi-netlink.h | 4 +
> net/ncsi/ncsi-rsp.c | 70 ++--
> 8 files changed, 420 insertions(+), 58 deletions(-)
>
> diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> index 4c292ec..4992bfc 100644
> --- a/include/uapi/linux/ncsi.h
> +++ b/include/uapi/linux/ncsi.h
> @@ -30,6 +30,7 @@ enum ncsi_nl_commands {
> NCSI_CMD_PKG_INFO,
> NCSI_CMD_SET_INTERFACE,
> NCSI_CMD_CLEAR_INTERFACE,
> + NCSI_CMD_SEND_CMD,
>
> __NCSI_CMD_AFTER_LAST,
> NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> @@ -43,6 +44,7 @@ enum ncsi_nl_commands {
> * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> * @NCSI_ATTR_PACKAGE_ID: package ID
> * @NCSI_ATTR_CHANNEL_ID: channel ID
> + * @NCSI_ATTR_DATA: command payload
> * @NCSI_ATTR_MAX: highest attribute number
> */
> enum ncsi_nl_attrs {
> @@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
> NCSI_ATTR_PACKAGE_LIST,
> NCSI_ATTR_PACKAGE_ID,
> NCSI_ATTR_CHANNEL_ID,
> + NCSI_ATTR_DATA,
>
> __NCSI_ATTR_AFTER_LAST,
> NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 8055e39..20ce735 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -215,12 +215,17 @@ struct ncsi_request {
> unsigned charid; /* Request ID - 0 to 255 */
> bool used;/* Request that has been assigned */
> unsigned int flags; /* NCSI request property */
> -#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> +#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
> struct ncsi_dev_priv *ndp;/* Associated NCSI device */
> struct sk_buff *cmd;/* Associated NCSI command packet */
> struct sk_buff *rsp;/* Associated NCSI response packet */
> struct timer_listtimer; /* Timer on waiting for response */
> bool enabled; /* Time has been enabled or not*/
> +
> + u32 snd_seq; /* netlink sending sequence number */
> + u32 snd_portid; /* netlink portid of sender*/
> + struct nlmsghdr nlhdr; /* netlink message header */
> };
>
> enum {
> @@ -301,10 +306,13 @@ struct ncsi_cmd_arg {
> unsigned short payload; /* Command packet payload length */
> unsigned int req_flags; /* NCSI request properties */
> union {
> - unsigned char bytes[16]; /* Command packet specific data */
> + unsigned char bytes[16]; /* Command packet specific data
> */
> unsigned short words[8];
> unsigned int dwords[4];
>
Re: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
On Thu, 2018-09-27 at 13:43 +1000, Samuel Mendoza-Jonas wrote:
> On Mon, 2018-09-24 at 17:08 -0700, Vijay Khemka wrote:
> > This patch adds OEM command to get mac address from NCSI device and and
> > configure the same to the network card.
> >
> > ncsi_cmd_arg - Modified this structure to include bigger payload data.
> > ncsi_cmd_handler_oem: This function handles oem command request
> > ncsi_rsp_handler_oem: This function handles response for OEM command.
> > get_mac_address_oem_mlx: This function will send OEM command to get
> > mac address for Mellanox card
> > set_mac_affinity_mlx: This will send OEM command to set Mac affinity
> > for Mellanox card
> >
> > Signed-off-by: Vijay Khemka
>
> Hi Vijay,
>
> Having had a chance to take a closer look, there is probably room for
> both this patchset and Justin's potential changes to coexist; while
> Justin's is a more general solution for sending arbitrary commands, the
> approach of this patch is also useful for handling commands we want
> included in the configure process (such as get-mac-address).
>
> Some comments below:
Whoops, forgot to re-add netdev.
>
> > ---
> > net/ncsi/Kconfig | 3 ++
> > net/ncsi/internal.h| 11 +--
> > net/ncsi/ncsi-cmd.c| 24 +--
> > net/ncsi/ncsi-manage.c | 68 ++
> > net/ncsi/ncsi-pkt.h| 16 ++
> > net/ncsi/ncsi-rsp.c| 33 +++-
> > 6 files changed, 149 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> > index 08a8a6031fd7..b8bf89fea7c8 100644
> > --- a/net/ncsi/Kconfig
> > +++ b/net/ncsi/Kconfig
> > @@ -10,3 +10,6 @@ config NET_NCSI
> > support. Enable this only if your system connects to a network
> > device via NCSI and the ethernet driver you're using supports
> > the protocol explicitly.
> > +config NCSI_OEM_CMD_GET_MAC
> > + bool "Get NCSI OEM MAC Address"
> > + depends on NET_NCSI
>
> For the moment this isn't too bad but I wonder if in the future it would
> be more flexible to have something like NCSI_OEM_CMD_MELLANOX etc, so we
> could selectively enable a class of OEM commands based on vendor rather
> than per-command.
>
> > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> > index 8055e3965cef..da17958e6a4b 100644
> > --- a/net/ncsi/internal.h
> > +++ b/net/ncsi/internal.h
> > @@ -68,6 +68,10 @@ enum {
> > NCSI_MODE_MAX
> > };
> >
> > +#define NCSI_OEM_MFR_MLX_ID 0x8119
> > +#define NCSI_OEM_MLX_CMD_GET_MAC0x1b00
> > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700
>
> I gather this is part of the OEM command but it would be good to describe
> what these bits mean. Is this command documented anywhere by Mellanox?
>
> > +
> > struct ncsi_channel_version {
> > u32 version;/* Supported BCD encoded NCSI version */
> > u32 alpha2; /* Supported BCD encoded NCSI version */
> > @@ -236,6 +240,7 @@ enum {
> > ncsi_dev_state_probe_dp,
> > ncsi_dev_state_config_sp= 0x0301,
> > ncsi_dev_state_config_cis,
> > + ncsi_dev_state_config_oem_gma,
> > ncsi_dev_state_config_clear_vids,
> > ncsi_dev_state_config_svf,
> > ncsi_dev_state_config_ev,
> > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
> > unsigned short payload; /* Command packet payload length */
> > unsigned int req_flags; /* NCSI request properties */
> > union {
> > - unsigned char bytes[16]; /* Command packet specific data */
> > - unsigned short words[8];
> > - unsigned int dwords[4];
> > + unsigned char bytes[64]; /* Command packet specific data */
> > + unsigned short words[32];
> > + unsigned int dwords[16];
> > };
> > };
> >
> > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> > index 7567ca63aae2..3205e22c1734 100644
> > --- a/net/ncsi/ncsi-cmd.c
> > +++ b/net/ncsi/ncsi-cmd.c
> > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> > return 0;
> > }
> >
> > +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> > + struct ncsi_cmd_arg *nca)
> > +{
> > + struct ncsi_cmd_oem_pkt *cmd;
> > + unsigned int len;
> > +
> > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> > + if (nca->payload < 26)
> > +
Re: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
On Tue, 2018-09-25 at 18:16 +, Vijay Khemka wrote:
> Hi Joel,
> Thanks, I am adding netdev mailing list here.
> Yes, this command is supported for all Mellanox card. It is as per Mellanox
> specification.
>
> Regards
> -Vijay
Hi Vijay,
Thanks for the patch; before I get too into a review though I'd like to
loop in Justin (cc'd) who I know is also working on an OEM command patch.
The changes here are very specific (eg. a command specific config option
"CONFIG_NCSI_OEM_CMD_GET_MAC"), which is ok on a small scale but if we
start to add an increasing amount of commands could get out of hand.
As I understand Justin's version adds a generic handler, using the NCSI
Netlink interface to pass OEM commands and responses to and from
userspace, which does the actual packet handling.
It would be good to compare these two approaches first before committing
to any one path
Justin - could you weigh in here and give a description of your intended
changes? Are you able to post your changes upstream so we can compare?
Regards,
Samuel
>
> On 9/24/18, 5:30 PM, "Joel Stanley" wrote:
>
> Hi Vijay,
>
> On Tue, 25 Sep 2018 at 09:39, Vijay Khemka wrote:
> >
> > This patch adds OEM command to get mac address from NCSI device and and
> > configure the same to the network card.
> >
> > ncsi_cmd_arg - Modified this structure to include bigger payload data.
> > ncsi_cmd_handler_oem: This function handes oem command request
> > ncsi_rsp_handler_oem: This function handles response for OEM command.
> > get_mac_address_oem_mlx: This function will send OEM command to get
> > mac address for Mellanox card
> > set_mac_affinity_mlx: This will send OEM command to set Mac affinity
> > for Mellanox card
>
> Thanks for the patch. The code looks okay, but I wanted to get some
> input from our NCSI maintainer as to how OEM commands would be
> structured. Sam, can you please provide some review here?
>
> Is the command supported on all melanox cards, just some, or does
> TiogaPass have a special firmware that enables it?
>
> We should include the netdev mailing list in this discussion as the
> patch needs to be acceptable for upstream.
>
> Cheers,
>
> Joel
>
> >
> > Signed-off-by: Vijay Khemka
> > ---
> > net/ncsi/Kconfig | 3 ++
> > net/ncsi/internal.h| 11 +--
> > net/ncsi/ncsi-cmd.c| 24 +--
> > net/ncsi/ncsi-manage.c | 68 ++
> > net/ncsi/ncsi-pkt.h| 16 ++
> > net/ncsi/ncsi-rsp.c| 33 +++-
> > 6 files changed, 149 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> > index 08a8a6031fd7..b8bf89fea7c8 100644
> > --- a/net/ncsi/Kconfig
> > +++ b/net/ncsi/Kconfig
> > @@ -10,3 +10,6 @@ config NET_NCSI
> > support. Enable this only if your system connects to a network
> > device via NCSI and the ethernet driver you're using supports
> > the protocol explicitly.
> > +config NCSI_OEM_CMD_GET_MAC
> > + bool "Get NCSI OEM MAC Address"
> > + depends on NET_NCSI
> > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> > index 8055e3965cef..da17958e6a4b 100644
> > --- a/net/ncsi/internal.h
> > +++ b/net/ncsi/internal.h
> > @@ -68,6 +68,10 @@ enum {
> > NCSI_MODE_MAX
> > };
> >
> > +#define NCSI_OEM_MFR_MLX_ID 0x8119
> > +#define NCSI_OEM_MLX_CMD_GET_MAC0x1b00
> > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700
> > +
> > struct ncsi_channel_version {
> > u32 version;/* Supported BCD encoded NCSI version */
> > u32 alpha2; /* Supported BCD encoded NCSI version */
> > @@ -236,6 +240,7 @@ enum {
> > ncsi_dev_state_probe_dp,
> > ncsi_dev_state_config_sp= 0x0301,
> > ncsi_dev_state_config_cis,
> > + ncsi_dev_state_config_oem_gma,
> > ncsi_dev_state_config_clear_vids,
> > ncsi_dev_state_config_svf,
> > ncsi_dev_state_config_ev,
> > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
> > unsigned short payload; /* Command packet payload
> length */
> > unsigned int req_flags; /* NCSI request properties
>*
Re: [PATCH 4/4] MAINTAINERS: Add Sam as the maintainer for NCSI
On Mon, 2018-06-18 at 16:49 +0930, Joel Stanley wrote: > Sam has been handing the maintenance of NCSI for a number release cycles > now. > > Signed-off-by: Joel Stanley I'm exposed! Acked-by: Samuel Mendoza-Jonas > --- > MAINTAINERS | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9d5eeff51b5f..44851f7c46fc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9756,6 +9756,11 @@ L: linux-s...@vger.kernel.org > S: Maintained > F: drivers/scsi/NCR_D700.* > > +NCSI LIBRARY: > +M: Samuel Mendoza-Jonas > +S: Maintained > +F: net/ncsi/ > + > NCT6775 HARDWARE MONITOR DRIVER > M: Guenter Roeck > L: linux-hw...@vger.kernel.org
Re: [PATCH 3/4] net/ncsi: Use netdev_dbg for debug messages
On Mon, 2018-06-18 at 13:53 -0700, Joe Perches wrote:
> On Mon, 2018-06-18 at 16:49 +0930, Joel Stanley wrote:
> > This moves all of the netdev_printk(KERN_DEBUG, ...) messages over to
> > netdev_dbg. There is no change in behaviour.
>
> Not quite, but I think the patch is fine anyway.
>
> netdev_printk(KERN_DEBUG ... is always emitted as
> long as the console level includes debug output.
>
> netdev_dbg is not included in object code unless
> DEBUG is defined or CONFIG_DYNAMIC_DEBUG is set.
> And then, it is not emitted into the log unless
> DEBUG is set or this specific netdev_dbg is enabled
> via the dynamic debug control file.
Right this is fine for these sort of messages; very noisy and not particularly
critical.
For this and the other logging updates:
Acked-by: Samuel Mendoza-Jonas
>
> > Signed-off-by: Joel Stanley
> > ---
> > net/ncsi/ncsi-aen.c| 6 +++---
> > net/ncsi/ncsi-manage.c | 33 +++--
> > 2 files changed, 18 insertions(+), 21 deletions(-)
> >
> > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> > index f899ed61bb57..25e483e8278b 100644
> > --- a/net/ncsi/ncsi-aen.c
> > +++ b/net/ncsi/ncsi-aen.c
> > @@ -148,9 +148,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv
> > *ndp,
> > hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
> > ncm->data[3] = ntohl(hncdsc->status);
> > spin_unlock_irqrestore(&nc->lock, flags);
> > - netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > - "NCSI: host driver %srunning on channel %u\n",
> > - ncm->data[3] & 0x1 ? "" : "not ", nc->id);
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: host driver %srunning on channel %u\n",
> > + ncm->data[3] & 0x1 ? "" : "not ", nc->id);
> >
> > return 0;
> > }
> > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> > index 716493a61ba6..091284760d21 100644
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -788,8 +788,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv
> > *ndp)
> > }
> > break;
> > case ncsi_dev_state_config_done:
> > - netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > - "NCSI: channel %u config done\n", nc->id);
> > + netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
> > + nc->id);
> > spin_lock_irqsave(&nc->lock, flags);
> > if (nc->reconfigure_needed) {
> > /* This channel's configuration has been updated
> > @@ -804,8 +804,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv
> > *ndp)
> > list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> > spin_unlock_irqrestore(&ndp->lock, flags);
> >
> > - netdev_printk(KERN_DEBUG, dev,
> > - "Dirty NCSI channel state reset\n");
> > + netdev_dbg(dev, "Dirty NCSI channel state reset\n");
> > ncsi_process_next_channel(ndp);
> > break;
> > }
> > @@ -908,9 +907,9 @@ static int ncsi_choose_active_channel(struct
> > ncsi_dev_priv *ndp)
> > }
> >
> > ncm = &found->modes[NCSI_MODE_LINK];
> > - netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > - "NCSI: Channel %u added to queue (link %s)\n",
> > - found->id, ncm->data[2] & 0x1 ? "up" : "down");
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: Channel %u added to queue (link %s)\n",
> > + found->id, ncm->data[2] & 0x1 ? "up" : "down");
> >
> > out:
> > spin_lock_irqsave(&ndp->lock, flags);
> > @@ -1316,9 +1315,9 @@ static int ncsi_kick_channels(struct ncsi_dev_priv
> > *ndp)
> > if ((ndp->ndev.state & 0xff00) ==
> > ncsi_dev_state_config ||
> > !list_empty(&nc->link)) {
> > - netdev_printk(KERN_DEBUG, nd->dev,
> > - "NCSI: channel %p marked
> > dirty\n",
> > -
[PATCH RESEND net-next] net/ncsi: Refactor MAC, VLAN filters
The NCSI driver defines a generic ncsi_channel_filter struct that can be
used to store arbitrarily formatted filters, and several generic methods
of accessing data stored in such a filter.
However in both the driver and as defined in the NCSI specification
there are only two actual filters: VLAN ID filters and MAC address
filters. The splitting of the MAC filter into unicast, multicast, and
mixed is also technically not necessary as these are stored in the same
location in hardware.
To save complexity, particularly in the set up and accessing of these
generic filters, remove them in favour of two specific structs. These
can be acted on directly and do not need several generic helper
functions to use.
This also fixes a memory error found by KASAN on ARM32 (which is not
upstream yet), where response handlers accessing a filter's data field
could write past allocated memory.
[ 114.926512]
==
[ 114.933861] BUG: KASAN: slab-out-of-bounds in
ncsi_configure_channel+0x4b8/0xc58
[ 114.941304] Read of size 2 at addr 94888558 by task kworker/0:2/546
[ 114.947593]
[ 114.949146] CPU: 0 PID: 546 Comm: kworker/0:2 Not tainted
4.16.0-rc6-00119-ge156398bfcad #13
...
[ 115.170233] The buggy address belongs to the object at 94888540
[ 115.170233] which belongs to the cache kmalloc-32 of size 32
[ 115.181917] The buggy address is located 24 bytes inside of
[ 115.181917] 32-byte region [94888540, 94888560)
[ 115.192115] The buggy address belongs to the page:
[ 115.196943] page:9eeac100 count:1 mapcount:0 mapping:94888000
index:0x94888fc1
[ 115.204200] flags: 0x100(slab)
[ 115.207330] raw: 0100 94888000 94888fc1 003f 0001 9eea2014
9eecaa74 96c003e0
[ 115.215444] page dumped because: kasan: bad access detected
[ 115.221036]
[ 115.222544] Memory state around the buggy address:
[ 115.227384] 94888400: fb fb fb fb fc fc fc fc 04 fc fc fc fc fc fc fc
[ 115.233959] 94888480: 00 00 00 fc fc fc fc fc 00 04 fc fc fc fc fc fc
[ 115.240529] >94888500: 00 00 04 fc fc fc fc fc 00 00 04 fc fc fc fc fc
[ 115.247077] ^
[ 115.252523] 94888580: 00 04 fc fc fc fc fc fc 06 fc fc fc fc fc fc fc
[ 115.259093] 94888600: 00 00 06 fc fc fc fc fc 00 00 04 fc fc fc fc fc
[ 115.265639]
==
Reported-by: Joel Stanley
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/internal.h | 34 +++---
net/ncsi/ncsi-manage.c | 226 +---
net/ncsi/ncsi-netlink.c | 20 ++--
net/ncsi/ncsi-rsp.c | 178 +--
4 files changed, 147 insertions(+), 311 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8da84312cd3b..8055e3965cef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -68,15 +68,6 @@ enum {
NCSI_MODE_MAX
};
-enum {
- NCSI_FILTER_BASE= 0,
- NCSI_FILTER_VLAN= 0,
- NCSI_FILTER_UC,
- NCSI_FILTER_MC,
- NCSI_FILTER_MIXED,
- NCSI_FILTER_MAX
-};
-
struct ncsi_channel_version {
u32 version;/* Supported BCD encoded NCSI version */
u32 alpha2; /* Supported BCD encoded NCSI version */
@@ -98,11 +89,18 @@ struct ncsi_channel_mode {
u32 data[8];/* Data entries*/
};
-struct ncsi_channel_filter {
- u32 index; /* Index of channel filters */
- u32 total; /* Total entries in the filter table */
- u64 bitmap; /* Bitmap of valid entries */
- u32 data[]; /* Data for the valid entries*/
+struct ncsi_channel_mac_filter {
+ u8 n_uc;
+ u8 n_mc;
+ u8 n_mixed;
+ u64 bitmap;
+ unsigned char *addrs;
+};
+
+struct ncsi_channel_vlan_filter {
+ u8 n_vids;
+ u64 bitmap;
+ u16 *vids;
};
struct ncsi_channel_stats {
@@ -186,7 +184,9 @@ struct ncsi_channel {
struct ncsi_channel_version version;
struct ncsi_channel_cap caps[NCSI_CAP_MAX];
struct ncsi_channel_modemodes[NCSI_MODE_MAX];
- struct ncsi_channel_filter *filters[NCSI_FILTER_MAX];
+ /* Filtering Settings */
+ struct ncsi_channel_mac_filter mac_filter;
+ struct ncsi_channel_vlan_filter vlan_filter;
struct ncsi_channel_stats stats;
struct {
struct timer_list timer;
@@ -320,10 +320,6 @@ extern spinlock_t ncsi_dev_lock;
list_for_each_entry_rcu(nc, &np->channels, node)
/* Resources */
-u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index);
-int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data);
-int ncsi_add_filter(struct ncsi_channel *nc, int table, void *data);
-int ncsi_remove_filter(struct ncsi_channel *nc, int table, int index);
void ncsi_start_channel_monitor(struct ncsi_chan
[PATCH net-next] net/ncsi: Refactor MAC, VLAN filters
The NCSI driver defines a generic ncsi_channel_filter struct that can be
used to store arbitrarily formatted filters, and several generic methods
of accessing data stored in such a filter.
However in both the driver and as defined in the NCSI specification
there are only two actual filters: VLAN ID filters and MAC address
filters. The splitting of the MAC filter into unicast, multicast, and
mixed is also technically not necessary as these are stored in the same
location in hardware.
To save complexity, particularly in the set up and accessing of these
generic filters, remove them in favour of two specific structs. These
can be acted on directly and do not need several generic helper
functions to use.
This also fixes a memory error found by KASAN on ARM32 (which is not
upstream yet), where response handlers accessing a filter's data field
could write past allocated memory.
[ 114.926512]
==
[ 114.933861] BUG: KASAN: slab-out-of-bounds in
ncsi_configure_channel+0x4b8/0xc58
[ 114.941304] Read of size 2 at addr 94888558 by task kworker/0:2/546
[ 114.947593]
[ 114.949146] CPU: 0 PID: 546 Comm: kworker/0:2 Not tainted
4.16.0-rc6-00119-ge156398bfcad #13
...
[ 115.170233] The buggy address belongs to the object at 94888540
[ 115.170233] which belongs to the cache kmalloc-32 of size 32
[ 115.181917] The buggy address is located 24 bytes inside of
[ 115.181917] 32-byte region [94888540, 94888560)
[ 115.192115] The buggy address belongs to the page:
[ 115.196943] page:9eeac100 count:1 mapcount:0 mapping:94888000
index:0x94888fc1
[ 115.204200] flags: 0x100(slab)
[ 115.207330] raw: 0100 94888000 94888fc1 003f 0001 9eea2014
9eecaa74 96c003e0
[ 115.215444] page dumped because: kasan: bad access detected
[ 115.221036]
[ 115.222544] Memory state around the buggy address:
[ 115.227384] 94888400: fb fb fb fb fc fc fc fc 04 fc fc fc fc fc fc fc
[ 115.233959] 94888480: 00 00 00 fc fc fc fc fc 00 04 fc fc fc fc fc fc
[ 115.240529] >94888500: 00 00 04 fc fc fc fc fc 00 00 04 fc fc fc fc fc
[ 115.247077] ^
[ 115.252523] 94888580: 00 04 fc fc fc fc fc fc 06 fc fc fc fc fc fc fc
[ 115.259093] 94888600: 00 00 06 fc fc fc fc fc 00 00 04 fc fc fc fc fc
[ 115.265639]
==
Reported-by: Joel Stanley
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/internal.h | 34 +++---
net/ncsi/ncsi-manage.c | 226 +---
net/ncsi/ncsi-netlink.c | 20 ++--
net/ncsi/ncsi-rsp.c | 178 +--
4 files changed, 147 insertions(+), 311 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8da84312cd3b..8055e3965cef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -68,15 +68,6 @@ enum {
NCSI_MODE_MAX
};
-enum {
- NCSI_FILTER_BASE= 0,
- NCSI_FILTER_VLAN= 0,
- NCSI_FILTER_UC,
- NCSI_FILTER_MC,
- NCSI_FILTER_MIXED,
- NCSI_FILTER_MAX
-};
-
struct ncsi_channel_version {
u32 version;/* Supported BCD encoded NCSI version */
u32 alpha2; /* Supported BCD encoded NCSI version */
@@ -98,11 +89,18 @@ struct ncsi_channel_mode {
u32 data[8];/* Data entries*/
};
-struct ncsi_channel_filter {
- u32 index; /* Index of channel filters */
- u32 total; /* Total entries in the filter table */
- u64 bitmap; /* Bitmap of valid entries */
- u32 data[]; /* Data for the valid entries*/
+struct ncsi_channel_mac_filter {
+ u8 n_uc;
+ u8 n_mc;
+ u8 n_mixed;
+ u64 bitmap;
+ unsigned char *addrs;
+};
+
+struct ncsi_channel_vlan_filter {
+ u8 n_vids;
+ u64 bitmap;
+ u16 *vids;
};
struct ncsi_channel_stats {
@@ -186,7 +184,9 @@ struct ncsi_channel {
struct ncsi_channel_version version;
struct ncsi_channel_cap caps[NCSI_CAP_MAX];
struct ncsi_channel_modemodes[NCSI_MODE_MAX];
- struct ncsi_channel_filter *filters[NCSI_FILTER_MAX];
+ /* Filtering Settings */
+ struct ncsi_channel_mac_filter mac_filter;
+ struct ncsi_channel_vlan_filter vlan_filter;
struct ncsi_channel_stats stats;
struct {
struct timer_list timer;
@@ -320,10 +320,6 @@ extern spinlock_t ncsi_dev_lock;
list_for_each_entry_rcu(nc, &np->channels, node)
/* Resources */
-u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index);
-int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data);
-int ncsi_add_filter(struct ncsi_channel *nc, int table, void *data);
-int ncsi_remove_filter(struct ncsi_channel *nc, int table, int index);
void ncsi_start_channel_monitor(struct ncsi_chan
[PATCH net-next v2] net/ncsi: Add generic netlink family
Add a generic netlink family for NCSI. This supports three commands;
NCSI_CMD_PKG_INFO which returns information on packages and their
associated channels, NCSI_CMD_SET_INTERFACE which allows a specific
package or package/channel combination to be set as the preferred
choice, and NCSI_CMD_CLEAR_INTERFACE which clears any preferred setting.
Signed-off-by: Samuel Mendoza-Jonas
---
v2: Add a separate NCSI_CMD_CLEAR_INTERFACE command instead of allowing
missing attributes in NCSI_CMD_SET_INTERFACE.
include/uapi/linux/ncsi.h | 115 +
net/ncsi/Makefile | 2 +-
net/ncsi/internal.h | 3 +
net/ncsi/ncsi-manage.c| 30 +++-
net/ncsi/ncsi-netlink.c | 421 ++
net/ncsi/ncsi-netlink.h | 20 +++
6 files changed, 586 insertions(+), 5 deletions(-)
create mode 100644 include/uapi/linux/ncsi.h
create mode 100644 net/ncsi/ncsi-netlink.c
create mode 100644 net/ncsi/ncsi-netlink.h
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
new file mode 100644
index ..4c292ecbb748
--- /dev/null
+++ b/include/uapi/linux/ncsi.h
@@ -0,0 +1,115 @@
+/*
+ * Copyright Samuel Mendoza-Jonas, IBM Corporation 2018.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __UAPI_NCSI_NETLINK_H__
+#define __UAPI_NCSI_NETLINK_H__
+
+/**
+ * enum ncsi_nl_commands - supported NCSI commands
+ *
+ * @NCSI_CMD_UNSPEC: unspecified command to catch errors
+ * @NCSI_CMD_PKG_INFO: list package and channel attributes. Requires
+ * NCSI_ATTR_IFINDEX. If NCSI_ATTR_PACKAGE_ID is specified returns the
+ * specific package and its channels - otherwise a dump request returns
+ * all packages and their associated channels.
+ * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
+ * Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
+ * optionally the preferred NCSI_ATTR_CHANNEL_ID.
+ * @NCSI_CMD_CLEAR_INTERFACE: clear any preferred package/channel combination.
+ * Requires NCSI_ATTR_IFINDEX.
+ * @NCSI_CMD_MAX: highest command number
+ */
+enum ncsi_nl_commands {
+ NCSI_CMD_UNSPEC,
+ NCSI_CMD_PKG_INFO,
+ NCSI_CMD_SET_INTERFACE,
+ NCSI_CMD_CLEAR_INTERFACE,
+
+ __NCSI_CMD_AFTER_LAST,
+ NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_attrs - General NCSI netlink attributes
+ *
+ * @NCSI_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_ATTR_IFINDEX: ifindex of network device using NCSI
+ * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
+ * @NCSI_ATTR_PACKAGE_ID: package ID
+ * @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_attrs {
+ NCSI_ATTR_UNSPEC,
+ NCSI_ATTR_IFINDEX,
+ NCSI_ATTR_PACKAGE_LIST,
+ NCSI_ATTR_PACKAGE_ID,
+ NCSI_ATTR_CHANNEL_ID,
+
+ __NCSI_ATTR_AFTER_LAST,
+ NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_pkg_attrs - NCSI netlink package-specific attributes
+ *
+ * @NCSI_PKG_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_PKG_ATTR: nested array of package attributes
+ * @NCSI_PKG_ATTR_ID: package ID
+ * @NCSI_PKG_ATTR_FORCED: flag signifying a package has been set as preferred
+ * @NCSI_PKG_ATTR_CHANNEL_LIST: nested array of NCSI_CHANNEL_ATTR attributes
+ * @NCSI_PKG_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_pkg_attrs {
+ NCSI_PKG_ATTR_UNSPEC,
+ NCSI_PKG_ATTR,
+ NCSI_PKG_ATTR_ID,
+ NCSI_PKG_ATTR_FORCED,
+ NCSI_PKG_ATTR_CHANNEL_LIST,
+
+ __NCSI_PKG_ATTR_AFTER_LAST,
+ NCSI_PKG_ATTR_MAX = __NCSI_PKG_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_channel_attrs - NCSI netlink channel-specific attributes
+ *
+ * @NCSI_CHANNEL_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_CHANNEL_ATTR: nested array of channel attributes
+ * @NCSI_CHANNEL_ATTR_ID: channel ID
+ * @NCSI_CHANNEL_ATTR_VERSION_MAJOR: channel major version number
+ * @NCSI_CHANNEL_ATTR_VERSION_MINOR: channel minor version number
+ * @NCSI_CHANNEL_ATTR_VERSION_STR: channel version string
+ * @NCSI_CHANNEL_ATTR_LINK_STATE: channel link state flags
+ * @NCSI_CHANNEL_ATTR_ACTIVE: channels with this flag are in
+ * NCSI_CHANNEL_ACTIVE state
+ * @NCSI_CHANNEL_ATTR_FORCED: flag signifying a channel has been set as
+ * preferred
+ * @NCSI_CHANNEL_ATTR_VLAN_LIST: nested array of NCSI_CHANNEL_ATTR_VLAN_IDs
+ * @NCSI_CHANNEL_ATTR_VLAN_ID: VLAN ID being filtered on this channel
+ * @NCSI_CHANNEL_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_channel_attrs {
+ NCSI_CHANNEL_ATTR_UNSPEC,
+ NCSI_CHANNEL_ATTR,
+ NCSI_CHANNEL_ATTR_ID,
+ NCSI_CHANNEL_ATTR_VERSION_MAJOR
Re: [PATCH net-next] net/ncsi: Add generic netlink family
On Mon, 2018-02-26 at 11:31 -0500, David Miller wrote: > From: Samuel Mendoza-Jonas > Date: Fri, 23 Feb 2018 15:15:18 +1100 > > > + * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination. > > + * Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and > > + * optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are > > + * specified the setting is cleared. > > I think clearing the setting when the required attributes are missing > is dangerous behavior. > > It is ambiguous whether the user intended the setting to be cleared, > or was in error and forgot to supply the attribute due to a bug. Fair point - I'll change this to be an error and add a separate command to clear the setting explicitly. In that vein is having NCSI_ATTR_CHANNEL_ID as an optional parameter ambiguous enough to justify a separate command as well? Regards, Sam
[PATCH net-next] net/ncsi: Add generic netlink family
Add a generic netlink family for NCSI. This supports two commands;
NCSI_CMD_PKG_INFO which returns information on packages and their
associated channels, and NCSI_CMD_SET_INTERFACE which allows a specific
package or package/channel combination to be set as the preferred
choice.
Signed-off-by: Samuel Mendoza-Jonas
---
include/uapi/linux/ncsi.h | 113 +
net/ncsi/Makefile | 2 +-
net/ncsi/internal.h | 3 +
net/ncsi/ncsi-manage.c| 30 +++-
net/ncsi/ncsi-netlink.c | 394 ++
net/ncsi/ncsi-netlink.h | 20 +++
6 files changed, 557 insertions(+), 5 deletions(-)
create mode 100644 include/uapi/linux/ncsi.h
create mode 100644 net/ncsi/ncsi-netlink.c
create mode 100644 net/ncsi/ncsi-netlink.h
diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
new file mode 100644
index ..aecab3fb92df
--- /dev/null
+++ b/include/uapi/linux/ncsi.h
@@ -0,0 +1,113 @@
+/*
+ * Copyright Samuel Mendoza-Jonas, IBM Corporation 2018.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __UAPI_NCSI_NETLINK_H__
+#define __UAPI_NCSI_NETLINK_H__
+
+/**
+ * enum ncsi_nl_commands - supported NCSI commands
+ *
+ * @NCSI_CMD_UNSPEC: unspecified command to catch errors
+ * @NCSI_CMD_SET_INTERFACE: set preferred package and channel combination.
+ * Requires NCSI_ATTR_IFINDEX and the preferred NCSI_ATTR_PACKAGE_ID and
+ * optionally the preferred NCSI_ATTR_CHANNEL_ID. If neither IDs are
+ * specified the setting is cleared.
+ * @NCSI_CMD_PKG_INFO: list package and channel attributes. Requires
+ * NCSI_ATTR_IFINDEX. If NCSI_ATTR_PACKAGE_ID is specified returns the
+ * specific package and its channels - otherwise a dump request returns
+ * all packages and their associated channels.
+ * @NCSI_CMD_MAX: highest command number
+ */
+enum ncsi_nl_commands {
+ NCSI_CMD_UNSPEC,
+ NCSI_CMD_SET_INTERFACE,
+ NCSI_CMD_PKG_INFO,
+
+ __NCSI_CMD_AFTER_LAST,
+ NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_attrs - General NCSI netlink attributes
+ *
+ * @NCSI_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_ATTR_IFINDEX: ifindex of network device using NCSI
+ * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
+ * @NCSI_ATTR_PACKAGE_ID: package ID
+ * @NCSI_ATTR_CHANNEL_ID: channel ID
+ * @NCSI_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_attrs {
+ NCSI_ATTR_UNSPEC,
+ NCSI_ATTR_IFINDEX,
+ NCSI_ATTR_PACKAGE_LIST,
+ NCSI_ATTR_PACKAGE_ID,
+ NCSI_ATTR_CHANNEL_ID,
+
+ __NCSI_ATTR_AFTER_LAST,
+ NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_pkg_attrs - NCSI netlink package-specific attributes
+ *
+ * @NCSI_PKG_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_PKG_ATTR: nested array of package attributes
+ * @NCSI_PKG_ATTR_ID: package ID
+ * @NCSI_PKG_ATTR_FORCED: flag signifying a package has been set as preferred
+ * @NCSI_PKG_ATTR_CHANNEL_LIST: nested array of NCSI_CHANNEL_ATTR attributes
+ * @NCSI_PKG_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_pkg_attrs {
+ NCSI_PKG_ATTR_UNSPEC,
+ NCSI_PKG_ATTR,
+ NCSI_PKG_ATTR_ID,
+ NCSI_PKG_ATTR_FORCED,
+ NCSI_PKG_ATTR_CHANNEL_LIST,
+
+ __NCSI_PKG_ATTR_AFTER_LAST,
+ NCSI_PKG_ATTR_MAX = __NCSI_PKG_ATTR_AFTER_LAST - 1
+};
+
+/**
+ * enum ncsi_nl_channel_attrs - NCSI netlink channel-specific attributes
+ *
+ * @NCSI_CHANNEL_ATTR_UNSPEC: unspecified attributes to catch errors
+ * @NCSI_CHANNEL_ATTR: nested array of channel attributes
+ * @NCSI_CHANNEL_ATTR_ID: channel ID
+ * @NCSI_CHANNEL_ATTR_VERSION_MAJOR: channel major version number
+ * @NCSI_CHANNEL_ATTR_VERSION_MINOR: channel minor version number
+ * @NCSI_CHANNEL_ATTR_VERSION_STR: channel version string
+ * @NCSI_CHANNEL_ATTR_LINK_STATE: channel link state flags
+ * @NCSI_CHANNEL_ATTR_ACTIVE: channels with this flag are in
+ * NCSI_CHANNEL_ACTIVE state
+ * @NCSI_CHANNEL_ATTR_FORCED: flag signifying a channel has been set as
+ * preferred
+ * @NCSI_CHANNEL_ATTR_VLAN_LIST: nested array of NCSI_CHANNEL_ATTR_VLAN_IDs
+ * @NCSI_CHANNEL_ATTR_VLAN_ID: VLAN ID being filtered on this channel
+ * @NCSI_CHANNEL_ATTR_MAX: highest attribute number
+ */
+enum ncsi_nl_channel_attrs {
+ NCSI_CHANNEL_ATTR_UNSPEC,
+ NCSI_CHANNEL_ATTR,
+ NCSI_CHANNEL_ATTR_ID,
+ NCSI_CHANNEL_ATTR_VERSION_MAJOR,
+ NCSI_CHANNEL_ATTR_VERSION_MINOR,
+ NCSI_CHANNEL_ATTR_VERSION_STR,
+ NCSI_CHANNEL_ATTR_LINK_STATE,
+ NCSI_CHANNEL_ATTR_ACTIVE,
+ NCSI_CHANNEL_ATTR_FORCED,
+ NCSI_CHANNEL_ATTR_VLAN_LIST,
+ NCSI_CHANNEL_ATTR_VLAN_ID,
+
+ __NCSI_CHANNEL_ATTR_AFTER_LAST
Re: [RFC PATCH] net/ncsi: Add generic netlink family
On Thu, 2018-02-15 at 14:50 +1030, Joel Stanley wrote:
> Hey Sam,
>
> On Thu, Feb 15, 2018 at 2:00 PM, Samuel Mendoza-Jonas
> wrote:
> > Add a generic netlink family for NCSI. This supports two commands;
> > NCSI_CMD_PKG_INFO which returns information on packages and their
> > associated channels, and NCSI_CMD_SET_INTERFACE which allows a specific
> > package or package/channel combination to be set as the preferred
> > choice.
> >
> > Signed-off-by: Samuel Mendoza-Jonas
> > ---
> > Fielding an RFC first to gauge what sort of information people may want
> > out of an NCSI user API before it gets carved in stone. This RFC exposes
> > a few main things such as link state, active link, channel versions, and
> > active vlan ids, is there more that could be helpful in version 1 of the
> > UAPI?
> > The big drawcard here is of course the ability to set preferred packages
> > and channels so that the NCSI link can be bound to *only* port 0 for
> > example. If you have opinions about how this should function now is the
> > time to speak up :)
>
> I'd recommend ccing the netdev mailing list in addition to OpenBMC.
True, I was mostly thinking of checking in with the OpenBMC group but
I'll add in netdev now as well in case I've made a netlink faux pas :)
>
> I was chatting with Facebook people about some advanced uses of NCSI.
> I've added Sai to cc, hopefully he can loop in the right people.
>
> The advanced uses included per-manufacturer OEM commands for MAC
> address retrieval, firmware updates, and I heard someone mention temp
> sensors-over-NCSI. I'm not that all of those would fall into the
> category of a netlink API or not, but it's worth considering those
> requirements in your design, even if you're not implementing it at
> this stage.
Yep there's been some brainstorming about this as well. I suspect at
least a generic "send an NCSI packet with this header and these
parameters" Netlink command could be a good fit - that way the NCSI
driver is aware of any response frames instead of receiving unexpected
frames and throwing them away / logging an error.
OEM-specific stuff is probably best left out of the driver but with that
interface could be handled nicely.
>
> Would a netlink API mean we would have to write some userspace tools
> to perform this configuration? Do you have any code for that?
Yes and yes - I've got a little python script that matches this patch,
I'll link it once I've cleaned it up a little.
>
> Cheers,
>
> Joel
>
>
> >
> > include/uapi/linux/ncsi.h | 65
> > net/ncsi/Makefile | 2 +-
> > net/ncsi/internal.h | 3 +
> > net/ncsi/ncsi-manage.c| 28 +++-
> > net/ncsi/ncsi-netlink.c | 394
> > ++
> > net/ncsi/ncsi-netlink.h | 20 +++
> > 6 files changed, 508 insertions(+), 4 deletions(-)
> > create mode 100644 include/uapi/linux/ncsi.h
> > create mode 100644 net/ncsi/ncsi-netlink.c
> > create mode 100644 net/ncsi/ncsi-netlink.h
> >
> > diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> > new file mode 100644
> > index ..45de201569e3
> > --- /dev/null
> > +++ b/include/uapi/linux/ncsi.h
> > @@ -0,0 +1,65 @@
> > +/*
> > + * Copyright Samuel Mendoza-Jonas, IBM Corporation 2018.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#ifndef __UAPI_NCSI_NETLINK_H__
> > +#define __UAPI_NCSI_NETLINK_H__
> > +
> > +enum ncsi_nl_commands {
> > + NCSI_CMD_UNSPEC,
> > + NCSI_CMD_SET_INTERFACE,
> > + NCSI_CMD_PKG_INFO,
> > + NCSI_CMD_MAX,
> > +};
> > +
> > +#define NCSI_CMD_MAX (NCSI_CMD_MAX - 1)
> > +
> > +enum ncsi_nl_attrs {
> > + NCSI_ATTR_UNSPEC,
> > + NCSI_ATTR_IFINDEX,
> > + NCSI_ATTR_PACKAGE_LIST,
> > + NCSI_ATTR_PACKAGE_ID,
> > + NCSI_ATTR_CHANNEL_ID,
> > + NCSI_ATTR_MAX,
> > +};
> > +
> > +enum ncsi_nl_pkg_attrs {
> > + NCSI_PKG_ATTR_UNSPEC,
> > + NCSI_PKG_ATTR,
> > + NCSI_PKG_ATTR_ID,
> > + NCSI_PKG_ATTR_CHANNEL_LIST,
> > + NCSI_PKG_ATTR_MAX,
> > +};
> > +
> > +enum ncsi_nl_channel_attrs {
> > + NCSI_CHANNEL_ATTR_UNSPEC,
> > +
[PATCH net-next] net/ncsi: Don't take any action on HNCDSC AEN
The current HNCDSC handler takes the status flag from the AEN packet and
will update or change the current channel based on this flag and the
current channel status.
However the flag from the HNCDSC packet merely represents the host link
state. While the state of the host interface is potentially interesting
information it should not affect the state of the NCSI link. Indeed the
NCSI specification makes no mention of any recommended action related to
the host network controller driver state.
Update the HNCDSC handler to record the host network driver status but
take no other action.
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-aen.c | 35 +++
1 file changed, 3 insertions(+), 32 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 67e708e98ccf..e7b05de1e6d1 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -143,43 +143,14 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv
*ndp,
if (!nc)
return -ENODEV;
- /* If the channel is active one, we need reconfigure it */
spin_lock_irqsave(&nc->lock, flags);
ncm = &nc->modes[NCSI_MODE_LINK];
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
- netdev_info(ndp->ndev.dev, "NCSI: HNCDSC AEN - channel %u state %s\n",
- nc->id, ncm->data[3] & 0x3 ? "up" : "down");
- if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE) {
- spin_unlock_irqrestore(&nc->lock, flags);
- return 0;
- }
-
- spin_unlock_irqrestore(&nc->lock, flags);
- if (!(ndp->flags & NCSI_DEV_HWA) && !(ncm->data[3] & 0x1))
- ndp->flags |= NCSI_DEV_RESHUFFLE;
-
- /* If this channel is the active one and the link doesn't
-* work, we have to choose another channel to be active one.
-* The logic here is exactly similar to what we do when link
-* is down on the active channel.
-*
-* On the other hand, we need configure it when host driver
-* state on the active channel becomes ready.
-*/
- ncsi_stop_channel_monitor(nc);
-
- spin_lock_irqsave(&nc->lock, flags);
- nc->state = (ncm->data[3] & 0x1) ? NCSI_CHANNEL_INACTIVE :
- NCSI_CHANNEL_ACTIVE;
spin_unlock_irqrestore(&nc->lock, flags);
-
- spin_lock_irqsave(&ndp->lock, flags);
- list_add_tail_rcu(&nc->link, &ndp->channel_queue);
- spin_unlock_irqrestore(&ndp->lock, flags);
-
- ncsi_process_next_channel(ndp);
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "NCSI: host driver %srunning on channel %u\n",
+ ncm->data[3] & 0x1 ? "" : "not ", nc->id);
return 0;
}
--
2.15.1
[GIT] [4.15] NFC update
Hi David, This is the NFC pull request for 4.15. We have: - A new netlink command for explicitly deactivating NFC targets - i2c constification for all NFC drivers - One NFC device allocation error path fix The following changes since commit 2798b80b385384d51a81832556ee9ad25d175f9b: Merge branch 'eBPF-based-device-cgroup-controller' (2017-11-05 23:26:51 +0900) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git tags/nfc-next-4.15-1 for you to fetch changes up to 4d63adfe12dd9cb61ed8badb4d798955399048c2: NFC: Add NFC_CMD_DEACTIVATE_TARGET support (2017-11-10 00:03:39 +0100) Allen Pais (1): NFC: Convert timers to use timer_setup() Arvind Yadav (8): nfc: microread: constify i2c_device_id nfc: nfcmrvl: constify i2c_device_id nfc: nxp-nci: constify i2c_device_id nfc: pn533: constify i2c_device_id nfc: pn544: constify i2c_device_id nfc: s3fwrn5: constify i2c_device_id nfc: st-nci: constify i2c_device_id nfc: st21nfca: constify i2c_device_id Colin Ian King (2): nfc: s3fwrn5: make array match static const NFC: fdp: make struct nci_ops static Johan Hovold (1): NFC: fix device-allocation error return Mark Greer (2): NFC: digital: Abort cmd when deactivating target NFC: Add NFC_CMD_DEACTIVATE_TARGET support drivers/nfc/fdp/fdp.c | 2 +- drivers/nfc/microread/i2c.c| 2 +- drivers/nfc/nfcmrvl/i2c.c | 2 +- drivers/nfc/nxp-nci/i2c.c | 2 +- drivers/nfc/pn533/i2c.c| 2 +- drivers/nfc/pn544/i2c.c| 2 +- drivers/nfc/s3fwrn5/firmware.c | 2 +- drivers/nfc/s3fwrn5/i2c.c | 2 +- drivers/nfc/st-nci/i2c.c | 2 +- drivers/nfc/st21nfca/i2c.c | 2 +- include/uapi/linux/nfc.h | 2 ++ net/nfc/core.c | 10 -- net/nfc/digital_core.c | 1 + net/nfc/hci/core.c | 7 +++ net/nfc/hci/llc_shdlc.c| 23 +-- net/nfc/llcp_core.c| 14 ++ net/nfc/netlink.c | 29 + 17 files changed, 64 insertions(+), 42 deletions(-)
[PATCH net-next 2/2] net/ncsi: Don't return error on normal response
Several response handlers return EBUSY if the data corresponding to the
command/response pair is already set. There is no reason to return an
error here; the channel is advertising something as enabled because we
told it to enable it, and it's possible that the feature has been
enabled previously.
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-rsp.c | 31 ++-
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 58186c4102f0..efd933ff5570 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -146,7 +146,7 @@ static int ncsi_rsp_handler_ec(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_ENABLE];
if (ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 1;
return 0;
@@ -173,7 +173,7 @@ static int ncsi_rsp_handler_dc(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_ENABLE];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 0;
return 0;
@@ -217,7 +217,7 @@ static int ncsi_rsp_handler_ecnt(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_TX_ENABLE];
if (ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 1;
return 0;
@@ -239,7 +239,7 @@ static int ncsi_rsp_handler_dcnt(struct ncsi_request *nr)
ncm = &nc->modes[NCSI_MODE_TX_ENABLE];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
ncm->enable = 1;
return 0;
@@ -263,7 +263,7 @@ static int ncsi_rsp_handler_ae(struct ncsi_request *nr)
/* Check if the AEN has been enabled */
ncm = &nc->modes[NCSI_MODE_AEN];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to AEN configuration */
cmd = (struct ncsi_cmd_ae_pkt *)skb_network_header(nr->cmd);
@@ -382,7 +382,7 @@ static int ncsi_rsp_handler_ev(struct ncsi_request *nr)
/* Check if VLAN mode has been enabled */
ncm = &nc->modes[NCSI_MODE_VLAN];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to VLAN mode */
cmd = (struct ncsi_cmd_ev_pkt *)skb_network_header(nr->cmd);
@@ -409,7 +409,7 @@ static int ncsi_rsp_handler_dv(struct ncsi_request *nr)
/* Check if VLAN mode has been enabled */
ncm = &nc->modes[NCSI_MODE_VLAN];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to VLAN mode */
ncm->enable = 0;
@@ -455,13 +455,10 @@ static int ncsi_rsp_handler_sma(struct ncsi_request *nr)
bitmap = &ncf->bitmap;
if (cmd->at_e & 0x1) {
- if (test_and_set_bit(cmd->index, bitmap))
- return -EBUSY;
+ set_bit(cmd->index, bitmap);
memcpy(ncf->data + 6 * cmd->index, cmd->mac, 6);
} else {
- if (!test_and_clear_bit(cmd->index, bitmap))
- return -EBUSY;
-
+ clear_bit(cmd->index, bitmap);
memset(ncf->data + 6 * cmd->index, 0, 6);
}
@@ -485,7 +482,7 @@ static int ncsi_rsp_handler_ebf(struct ncsi_request *nr)
/* Check if broadcast filter has been enabled */
ncm = &nc->modes[NCSI_MODE_BC];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to broadcast filter mode */
cmd = (struct ncsi_cmd_ebf_pkt *)skb_network_header(nr->cmd);
@@ -511,7 +508,7 @@ static int ncsi_rsp_handler_dbf(struct ncsi_request *nr)
/* Check if broadcast filter isn't enabled */
ncm = &nc->modes[NCSI_MODE_BC];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to broadcast filter mode */
ncm->enable = 0;
@@ -538,7 +535,7 @@ static int ncsi_rsp_handler_egmf(struct ncsi_request *nr)
/* Check if multicast filter has been enabled */
ncm = &nc->modes[NCSI_MODE_MC];
if (ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to multicast filter mode */
cmd = (struct ncsi_cmd_egmf_pkt *)skb_network_header(nr->cmd);
@@ -564,7 +561,7 @@ static int ncsi_rsp_handler_dgmf(struct ncsi_request *nr)
/* Check if multicast filter has been enabled */
ncm = &nc->modes[NCSI_MODE_MC];
if (!ncm->enable)
- return -EBUSY;
+ return 0;
/* Update to multicast filter mode */
ncm->enable = 0;
@@ -591,7 +588,7 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
/* Check if flow control has been enabled */
ncm = &nc->modes[NCSI_MOD
[PATCH net-next 1/2] net/ncsi: Improve general state logging
The NCSI driver is mostly silent which becomes a headache when trying to
determine what has occurred on the NCSI connection. This adds additional
logging in a few key areas such as state transitions and calling out
certain errors more visibly.
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-aen.c| 15 +-
net/ncsi/ncsi-manage.c | 76 +-
net/ncsi/ncsi-rsp.c| 10 ++-
3 files changed, 80 insertions(+), 21 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index f135938bf781..67e708e98ccf 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -73,6 +73,9 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
ncm->data[2] = data;
ncm->data[4] = ntohl(lsc->oem_status);
+ netdev_info(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
+ nc->id, data & 0x1 ? "up" : "down");
+
chained = !list_empty(&nc->link);
state = nc->state;
spin_unlock_irqrestore(&nc->lock, flags);
@@ -145,6 +148,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv
*ndp,
ncm = &nc->modes[NCSI_MODE_LINK];
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
+ netdev_info(ndp->ndev.dev, "NCSI: HNCDSC AEN - channel %u state %s\n",
+ nc->id, ncm->data[3] & 0x3 ? "up" : "down");
if (!list_empty(&nc->link) ||
nc->state != NCSI_CHANNEL_ACTIVE) {
spin_unlock_irqrestore(&nc->lock, flags);
@@ -212,10 +217,18 @@ int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct
sk_buff *skb)
}
ret = ncsi_validate_aen_pkt(h, nah->payload);
- if (ret)
+ if (ret) {
+ netdev_warn(ndp->ndev.dev,
+ "NCSI: 'bad' packet ignored for AEN type 0x%x\n",
+ h->type);
goto out;
+ }
ret = nah->handler(ndp, h);
+ if (ret)
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Handler for AEN type 0x%x returned %d\n",
+ h->type, ret);
out:
consume_skb(skb);
return ret;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 47baf914eec2..a2b904a718c6 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -229,6 +229,8 @@ static void ncsi_channel_monitor(unsigned long data)
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
default:
+ netdev_err(ndp->ndev.dev, "NCSI Channel %d timed out!\n",
+ nc->id);
if (!(ndp->flags & NCSI_DEV_HWA)) {
ncsi_report_link(ndp, true);
ndp->flags |= NCSI_DEV_RESHUFFLE;
@@ -682,7 +684,7 @@ static int clear_one_vid(struct ncsi_dev_priv *ndp, struct
ncsi_channel *nc,
data = ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
if (!data) {
netdev_err(ndp->ndev.dev,
- "ncsi: failed to retrieve filter %d\n", index);
+ "NCSI: failed to retrieve filter %d\n", index);
/* Set the VLAN id to 0 - this will still disable the entry in
* the filter table, but we won't know what it was.
*/
@@ -692,7 +694,7 @@ static int clear_one_vid(struct ncsi_dev_priv *ndp, struct
ncsi_channel *nc,
}
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "ncsi: removed vlan tag %u at index %d\n",
+ "NCSI: removed vlan tag %u at index %d\n",
vid, index + 1);
ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
@@ -718,7 +720,7 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct
ncsi_channel *nc,
if (index < 0) {
/* New tag to add */
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "ncsi: new vlan id to set: %u\n",
+ "NCSI: new vlan id to set: %u\n",
vlan->vid);
break;
}
@@ -745,7 +747,7 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct
ncsi_channel *nc,
}
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "ncsi: set vid %u in packet, index %u\n",
+ "NCSI: set vid %u in packet, index %u\n",
vlan->vid, index + 1);
nca->type = NCSI_PKT_CMD_SVF;
nca->words[1] = vlan
Re: [PATCH] NFC: fix device-allocation error return
Hi Johan,
On Sun, Jul 09, 2017 at 01:08:58PM +0200, Johan Hovold wrote:
> A recent change fixing NFC device allocation itself introduced an
> error-handling bug by returning an error pointer in case device-id
> allocation failed. This is clearly broken as the callers still expected
> NULL to be returned on errors as detected by Dan's static checker.
>
> Fix this up by returning NULL in the event that we've run out of memory
> when allocating a new device id.
>
> Note that the offending commit is marked for stable (3.8) so this fix
> needs to be backported along with it.
>
> Fixes: 20777bc57c34 ("NFC: fix broken device allocation")
> Cc: stable# 3.8
> Reported-by: Dan Carpenter
> Signed-off-by: Johan Hovold
> ---
> net/nfc/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks for the fix.
Cheers,
Samuel.
[PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length
Correct the value of the HNCDSC AEN packet.
Fixes: 7a82ecf4cfb85 "net/ncsi: NCSI AEN packet handler"
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-aen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 6898e7229285..f135938bf781 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -187,7 +187,7 @@ static struct ncsi_aen_handler {
} ncsi_aen_handlers[] = {
{ NCSI_PKT_AEN_LSC,12, ncsi_aen_handler_lsc},
{ NCSI_PKT_AEN_CR, 4, ncsi_aen_handler_cr },
- { NCSI_PKT_AEN_HNCDSC, 4, ncsi_aen_handler_hncdsc }
+ { NCSI_PKT_AEN_HNCDSC, 8, ncsi_aen_handler_hncdsc }
};
int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct sk_buff *skb)
--
2.14.2
[PATCH net 3/5] net/ncsi: Disable HWA mode when no channels are found
From: Gavin Shan
When there are no NCSI channels probed, HWA (Hardware Arbitration)
mode is enabled. It's not correct because HWA depends on the fact:
NCSI channels exist and all of them support HWA mode. This disables
HWA when no channels are probed.
Signed-off-by: Gavin Shan
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-manage.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b022deb39d31..0966eff48ce7 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1005,12 +1005,15 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp)
struct ncsi_package *np;
struct ncsi_channel *nc;
unsigned int cap;
+ bool has_channel = false;
/* The hardware arbitration is disabled if any one channel
* doesn't support explicitly.
*/
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
+ has_channel = true;
+
cap = nc->caps[NCSI_CAP_GENERIC].cap;
if (!(cap & NCSI_CAP_GENERIC_HWA) ||
(cap & NCSI_CAP_GENERIC_HWA_MASK) !=
@@ -1021,8 +1024,13 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp)
}
}
- ndp->flags |= NCSI_DEV_HWA;
- return true;
+ if (has_channel) {
+ ndp->flags |= NCSI_DEV_HWA;
+ return true;
+ }
+
+ ndp->flags &= ~NCSI_DEV_HWA;
+ return false;
}
static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp)
--
2.14.2
[PATCH net 2/5] net/ncsi: Stop monitor if channel times out or is inactive
ncsi_channel_monitor() misses stopping the channel monitor in several
places that it should, causing a WARN_ON_ONCE() to trigger when the
monitor is re-started later, eg:
[ 459.04] WARNING: CPU: 0 PID: 1093 at net/ncsi/ncsi-manage.c:269
ncsi_start_channel_monitor+0x7c/0x90
[ 459.04] CPU: 0 PID: 1093 Comm: kworker/0:3 Not tainted 4.10.17-gaca2fdd
#140
[ 459.04] Hardware name: ASpeed SoC
[ 459.04] Workqueue: events ncsi_dev_work
[ 459.04] [<80010094>] (unwind_backtrace) from [<8000d950>]
(show_stack+0x20/0x24)
[ 459.04] [<8000d950>] (show_stack) from [<801dbf70>]
(dump_stack+0x20/0x28)
[ 459.04] [<801dbf70>] (dump_stack) from [<80018d7c>] (__warn+0xe0/0x108)
[ 459.04] [<80018d7c>] (__warn) from [<80018e70>]
(warn_slowpath_null+0x30/0x38)
[ 459.04] [<80018e70>] (warn_slowpath_null) from [<803f6a08>]
(ncsi_start_channel_monitor+0x7c/0x90)
[ 459.04] [<803f6a08>] (ncsi_start_channel_monitor) from [<803f7664>]
(ncsi_configure_channel+0xdc/0x5fc)
[ 459.04] [<803f7664>] (ncsi_configure_channel) from [<803f8160>]
(ncsi_dev_work+0xac/0x474)
[ 459.04] [<803f8160>] (ncsi_dev_work) from [<8002d244>]
(process_one_work+0x1e0/0x450)
[ 459.04] [<8002d244>] (process_one_work) from [<8002d510>]
(worker_thread+0x5c/0x570)
[ 459.04] [<8002d510>] (worker_thread) from [<80033614>]
(kthread+0x124/0x164)
[ 459.04] [<80033614>] (kthread) from [<8000a5e8>]
(ret_from_fork+0x14/0x2c)
This also updates the monitor instead of just returning if
ncsi_xmit_cmd() fails to send the get-link-status command so that the
monitor properly times out.
Fixes: e6f44ed6d04d3 "net/ncsi: Package and channel management"
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-manage.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b6a449aa9d4b..b022deb39d31 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -202,11 +202,15 @@ static void ncsi_channel_monitor(unsigned long data)
monitor_state = nc->monitor.state;
spin_unlock_irqrestore(&nc->lock, flags);
- if (!enabled || chained)
+ if (!enabled || chained) {
+ ncsi_stop_channel_monitor(nc);
return;
+ }
if (state != NCSI_CHANNEL_INACTIVE &&
- state != NCSI_CHANNEL_ACTIVE)
+ state != NCSI_CHANNEL_ACTIVE) {
+ ncsi_stop_channel_monitor(nc);
return;
+ }
switch (monitor_state) {
case NCSI_CHANNEL_MONITOR_START:
@@ -217,12 +221,9 @@ static void ncsi_channel_monitor(unsigned long data)
nca.type = NCSI_PKT_CMD_GLS;
nca.req_flags = 0;
ret = ncsi_xmit_cmd(&nca);
- if (ret) {
+ if (ret)
netdev_err(ndp->ndev.dev, "Error %d sending GLS\n",
ret);
- return;
- }
-
break;
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
@@ -233,6 +234,8 @@ static void ncsi_channel_monitor(unsigned long data)
ndp->flags |= NCSI_DEV_RESHUFFLE;
}
+ ncsi_stop_channel_monitor(nc);
+
spin_lock_irqsave(&nc->lock, flags);
nc->state = NCSI_CHANNEL_INVISIBLE;
spin_unlock_irqrestore(&nc->lock, flags);
--
2.14.2
[PATCH net 5/5] net/ncsi: Fix length of GVI response packet
From: Gavin Shan
The length of GVI (GetVersionInfo) response packet should be 40 instead
of 36. This issue was found from /sys/kernel/debug/ncsi/eth0/stats.
# ethtool --ncsi eth0 swstats
:
RESPONSE OK TIMEOUT ERROR
===
GVI 002
With this applied, no error reported on GVI response packets:
# ethtool --ncsi eth0 swstats
:
RESPONSE OK TIMEOUT ERROR
===
GVI 200
Signed-off-by: Gavin Shan
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-rsp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 265b9a892d41..927dad4759d1 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -959,7 +959,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_EGMF,4, ncsi_rsp_handler_egmf},
{ NCSI_PKT_RSP_DGMF,4, ncsi_rsp_handler_dgmf},
{ NCSI_PKT_RSP_SNFC,4, ncsi_rsp_handler_snfc},
- { NCSI_PKT_RSP_GVI,36, ncsi_rsp_handler_gvi },
+ { NCSI_PKT_RSP_GVI,40, ncsi_rsp_handler_gvi },
{ NCSI_PKT_RSP_GC, 32, ncsi_rsp_handler_gc },
{ NCSI_PKT_RSP_GP, -1, ncsi_rsp_handler_gp },
{ NCSI_PKT_RSP_GCPS, 172, ncsi_rsp_handler_gcps},
--
2.14.2
[PATCH net 4/5] net/ncsi: Enforce failover on link monitor timeout
From: Gavin Shan
The NCSI channel has been configured to provide service if its link
monitor timer is enabled, regardless of its state (inactive or active).
So the timeout event on the link monitor indicates the out-of-service
on that channel, for which a failover is needed.
This sets NCSI_DEV_RESHUFFLE flag to enforce failover on link monitor
timeout, regardless the channel's original state (inactive or active).
Also, the link is put into "down" state to give the failing channel
lowest priority when selecting for the active channel. The state of
failing channel should be set to active in order for deinitialization
and failover to be done.
Signed-off-by: Gavin Shan
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-manage.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0966eff48ce7..28c42b22b748 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -189,6 +189,7 @@ static void ncsi_channel_monitor(unsigned long data)
struct ncsi_channel *nc = (struct ncsi_channel *)data;
struct ncsi_package *np = nc->package;
struct ncsi_dev_priv *ndp = np->ndp;
+ struct ncsi_channel_mode *ncm;
struct ncsi_cmd_arg nca;
bool enabled, chained;
unsigned int monitor_state;
@@ -228,20 +229,21 @@ static void ncsi_channel_monitor(unsigned long data)
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
default:
- if (!(ndp->flags & NCSI_DEV_HWA) &&
- state == NCSI_CHANNEL_ACTIVE) {
+ if (!(ndp->flags & NCSI_DEV_HWA)) {
ncsi_report_link(ndp, true);
ndp->flags |= NCSI_DEV_RESHUFFLE;
}
ncsi_stop_channel_monitor(nc);
+ ncm = &nc->modes[NCSI_MODE_LINK];
spin_lock_irqsave(&nc->lock, flags);
nc->state = NCSI_CHANNEL_INVISIBLE;
+ ncm->data[2] &= ~0x1;
spin_unlock_irqrestore(&nc->lock, flags);
spin_lock_irqsave(&ndp->lock, flags);
- nc->state = NCSI_CHANNEL_INACTIVE;
+ nc->state = NCSI_CHANNEL_ACTIVE;
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
ncsi_process_next_channel(ndp);
--
2.14.2
[PATCH net] net/ncsi: Don't limit vids based on hot_channel
Currently we drop any new VLAN ids if there are more than the current
(or last used) channel can support. Most importantly this is a problem
if no channel has been selected yet, resulting in a segfault.
Secondly this does not necessarily reflect the capabilities of any other
channels. Instead only drop a new VLAN id if we are already tracking the
maximum allowed by the NCSI specification. Per-channel limits are
already handled by ncsi_add_filter(), but add a message to set_one_vid()
to make it obvious that the channel can not support any more VLAN ids.
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/internal.h| 1 +
net/ncsi/ncsi-manage.c | 17 +
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index af3d636534ef..d30f7bd741d0 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -286,6 +286,7 @@ struct ncsi_dev_priv {
struct work_struct work;/* For channel management */
struct packet_type ptype; /* NCSI packet Rx handler */
struct list_headnode;/* Form NCSI device list */
+#define NCSI_MAX_VLAN_VIDS 15
struct list_headvlan_vids; /* List of active VLAN IDs */
};
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 3fd3c39e6278..b6a449aa9d4b 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -732,6 +732,10 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct
ncsi_channel *nc,
if (index < 0) {
netdev_err(ndp->ndev.dev,
"Failed to add new VLAN tag, error %d\n", index);
+ if (index == -ENOSPC)
+ netdev_err(ndp->ndev.dev,
+ "Channel %u already has all VLAN filters
set\n",
+ nc->id);
return -1;
}
@@ -1403,7 +1407,6 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
{
- struct ncsi_channel_filter *ncf;
struct ncsi_dev_priv *ndp;
unsigned int n_vids = 0;
struct vlan_vid *vlan;
@@ -1420,7 +1423,6 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16
proto, u16 vid)
}
ndp = TO_NCSI_DEV_PRIV(nd);
- ncf = ndp->hot_channel->filters[NCSI_FILTER_VLAN];
/* Add the VLAN id to our internal list */
list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
@@ -1431,12 +1433,11 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16
proto, u16 vid)
return 0;
}
}
-
- if (n_vids >= ncf->total) {
- netdev_info(dev,
- "NCSI Channel supports up to %u VLAN tags but %u
are already set\n",
- ncf->total, n_vids);
- return -EINVAL;
+ if (n_vids >= NCSI_MAX_VLAN_VIDS) {
+ netdev_warn(dev,
+ "tried to add vlan id %u but NCSI max already
registered (%u)\n",
+ vid, NCSI_MAX_VLAN_VIDS);
+ return -ENOSPC;
}
vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
--
2.14.2
Re: [PATCH net] net/ncsi: Don't assume last available channel exists
On Thu, 2017-09-21 at 18:11 -0700, David Miller wrote: > From: Samuel Mendoza-Jonas > Date: Fri, 22 Sep 2017 11:00:00 +1000 > > > If we haven't configured a channel yet (or are in the process of doing > > so) we won't have a hot_channel - does it make more sense to > > - check against the hot_channel as currently done, > > - only check the filter size at configure time for /each/ channel, > > - only conditionally enable the .ndo_vlan_rx_add_vid net_device callback > > once we've configured a channel (eg. for ftgmac100 in the > > ftgmac100_ncsi_handler() callback?) > > The last isn't so feasible. > > The device shouldn't be marked attached until a channel is available, > because it seems like communication cannot occur until one is. Right? Yes that's right. > > You could experiment with netif_device_detach()/netif_device_attach(). > > When the device is in the detached state, callbacks such as > ->ndo_vlan_rx_add_vid() will not be invoked. This looked like the way at first, but _detach() ceases any tx/rx on the interface right? NCSI still needs the interface to be active since the 'channels' are on a separate network controller that the interface is connected to, eg on the machines I'm using: BMC 'Host' network controller -- |ftgmac100 interface | < NCSI Link > | BCM5719 interface| --> external interface -- Looking at the NCSI init path I believe we're guaranteed to have an ndp struct by the time ndo_vlan_rx_add_vid() is called, making some of those checks overly cautious. It might be easiest to just track new vids as we see them (up to the NCSI spec limit), and then deal with configured channels on a case by case basis since their limits can be different. I'll work on a V2 but hopefully I haven't misinterpreted _detach()/_attach() :) Sam
Re: [PATCH net] net/ncsi: Don't assume last available channel exists
On Wed, 2017-09-20 at 16:05 -0700, David Miller wrote:
> From: Samuel Mendoza-Jonas
> Date: Wed, 20 Sep 2017 14:12:51 +1000
>
> > When handling new VLAN tags in NCSI we check the maximum allowed number
> > of filters on the last active ("hot") channel. However if the 'add'
> > callback is called before NCSI has configured a channel, this causes a
> > NULL dereference.
> >
> > Check that we actually have a hot channel, and warn if it is missing.
> >
> > Signed-off-by: Samuel Mendoza-Jonas
>
> Well, a few things.
>
> We should not allow this driver method to be invoked in the first
> place if the device is not in a state where the operation is legal
> yet.
>
> Second of all, if !ndp is true, you should not return 0 to indicate
> success.
>
> But more fundamentally, we should block this method from being
> callable unless the device is in the proper state and can complete the
> operation.
>
> Seriously, these checks should be completely unnecessary if those
> issues are handled properly.
Good point, this made me step back and reconsider the problem here.
The ncsi_vlan_rx_add_vid() callback exists because the NCSI driver needs
to know which VLAN IDs are set, but we don't have a straightforward way
of accessing that information later in ncsi_configure_channel() - as
opposed to the MAC address for example which is just accessed via
dev->dev_addr. Instead they're kept track of in the ndp->vlan_vids list
and the NCSI driver reconfigures any channels it knows about.
So in that sense the NCSI device *is* ready to handle the operation. The
hot_channel fix here was to check if we were exceeding the maximum number
of VLAN filters supported by the remote channel. That itself is bit
debatable since different channels could support different numbers of
filters but right now the NCSI driver only supports one active channel at
a time.
If we haven't configured a channel yet (or are in the process of doing
so) we won't have a hot_channel - does it make more sense to
- check against the hot_channel as currently done,
- only check the filter size at configure time for /each/ channel,
- only conditionally enable the .ndo_vlan_rx_add_vid net_device callback
once we've configured a channel (eg. for ftgmac100 in the
ftgmac100_ncsi_handler() callback?)
Thanks,
Sam
[PATCH net] net/ncsi: Don't assume last available channel exists
When handling new VLAN tags in NCSI we check the maximum allowed number
of filters on the last active ("hot") channel. However if the 'add'
callback is called before NCSI has configured a channel, this causes a
NULL dereference.
Check that we actually have a hot channel, and warn if it is missing.
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-manage.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 3fd3c39e6278..fc800f934beb 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1420,7 +1420,10 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16
proto, u16 vid)
}
ndp = TO_NCSI_DEV_PRIV(nd);
- ncf = ndp->hot_channel->filters[NCSI_FILTER_VLAN];
+ if (!ndp) {
+ netdev_warn(dev, "ncsi: No ncsi_dev_priv?\n");
+ return 0;
+ }
/* Add the VLAN id to our internal list */
list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
@@ -1432,11 +1435,17 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16
proto, u16 vid)
}
}
- if (n_vids >= ncf->total) {
- netdev_info(dev,
- "NCSI Channel supports up to %u VLAN tags but %u
are already set\n",
- ncf->total, n_vids);
- return -EINVAL;
+ if (!ndp->hot_channel) {
+ netdev_warn(dev,
+ "ncsi: no available filter to check maximum\n");
+ } else {
+ ncf = ndp->hot_channel->filters[NCSI_FILTER_VLAN];
+ if (n_vids >= ncf->total) {
+ netdev_info(dev,
+ "NCSI Channel supports up to %u VLAN tags
but %u are already set\n",
+ ncf->total, n_vids);
+ return -EINVAL;
+ }
}
vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
--
2.14.1
[PATCH net-next] net/ncsi: Define {add,kill}_vid callbacks for !CONFIG_NET_NCSI
Patch "net/ncsi: Configure VLAN tag filter" defined two new callback
functions in include/net/ncsi.h, but neglected the !CONFIG_NET_NCSI
case. This can cause a build error if these are referenced elsewhere
without NCSI enabled, for example in ftgmac100:
>>> ERROR: "ncsi_vlan_rx_kill_vid" [drivers/net/ethernet/faraday/ftgmac100.ko]
>>> undefined!
>>> ERROR: "ncsi_vlan_rx_add_vid" [drivers/net/ethernet/faraday/ftgmac100.ko]
>>> undefined!
Add definitions for !CONFIG_NET_NCSI to bring it into line with the rest
of ncsi.h
Signed-off-by: Samuel Mendoza-Jonas
---
include/net/ncsi.h | 8
1 file changed, 8 insertions(+)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 1f96af46df49..2b13b6b91a4d 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -36,6 +36,14 @@ int ncsi_start_dev(struct ncsi_dev *nd);
void ncsi_stop_dev(struct ncsi_dev *nd);
void ncsi_unregister_dev(struct ncsi_dev *nd);
#else /* !CONFIG_NET_NCSI */
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ return -ENOTTY;
+}
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ return -ENOTTY;
+}
static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd))
{
--
2.14.1
Re: [PATCH net-next v3 0/3] NCSI VLAN Filtering Support
On Mon, 2017-08-28 at 16:50 -0700, David Miller wrote:
> From: Samuel Mendoza-Jonas
> Date: Mon, 28 Aug 2017 16:18:40 +1000
>
> > This series (mainly patch 2) adds VLAN filtering to the NCSI implementation.
> > A fair amount of code already exists in the NCSI stack for VLAN filtering
> > but
> > none of it is actually hooked up. This goes the final mile and fixes a few
> > bugs in the existing code found along the way (patch 1).
> >
> > Patch 3 adds the appropriate flag and callbacks to the ftgmac100 driver to
> > enable filtering as it's a large consumer of NCSI (and what I've been
> > testing on).
> >
> > v3: - Add comment describing change to ncsi_find_filter()
> > - Catch NULL in clear_one_vid() from ncsi_get_filter()
> > - Simplify state changes when kicking updated channel
>
> Series applied.
Thanks David,
The kbuild bot caught a build error where the add/kill callbacks aren't
defined without CONFIG_NET_NCSI:
>> ERROR: "ncsi_vlan_rx_kill_vid" [drivers/net/ethernet/faraday/ftgmac100.ko]
>> undefined!
>> ERROR: "ncsi_vlan_rx_add_vid" [drivers/net/ethernet/faraday/ftgmac100.ko]
>> undefined!
It's a quick fixup to patch 3 as below, would you like me to send it as a v4?
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 1f96af46df49..2b13b6b91a4d 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -36,6 +36,14 @@ int ncsi_start_dev(struct ncsi_dev *nd);
void ncsi_stop_dev(struct ncsi_dev *nd);
void ncsi_unregister_dev(struct ncsi_dev *nd);
#else /* !CONFIG_NET_NCSI */
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ return -ENOTTY;
+}
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ return -ENOTTY;
+}
static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd))
{
[PATCH net-next v3 0/3] NCSI VLAN Filtering Support
This series (mainly patch 2) adds VLAN filtering to the NCSI implementation. A fair amount of code already exists in the NCSI stack for VLAN filtering but none of it is actually hooked up. This goes the final mile and fixes a few bugs in the existing code found along the way (patch 1). Patch 3 adds the appropriate flag and callbacks to the ftgmac100 driver to enable filtering as it's a large consumer of NCSI (and what I've been testing on). v3: - Add comment describing change to ncsi_find_filter() - Catch NULL in clear_one_vid() from ncsi_get_filter() - Simplify state changes when kicking updated channel Samuel Mendoza-Jonas (3): net/ncsi: Fix several packet definitions net/ncsi: Configure VLAN tag filter ftgmac100: Support NCSI VLAN filtering when available drivers/net/ethernet/faraday/ftgmac100.c | 5 + include/net/ncsi.h | 2 + net/ncsi/internal.h | 11 ++ net/ncsi/ncsi-cmd.c | 10 +- net/ncsi/ncsi-manage.c | 308 ++- net/ncsi/ncsi-pkt.h | 2 +- net/ncsi/ncsi-rsp.c | 12 +- 7 files changed, 339 insertions(+), 11 deletions(-) -- 2.14.0
[PATCH net-next v3 1/3] net/ncsi: Fix several packet definitions
Signed-off-by: Samuel Mendoza-Jonas
---
v2: Rebased on latest net-next
net/ncsi/ncsi-cmd.c | 10 +-
net/ncsi/ncsi-pkt.h | 2 +-
net/ncsi/ncsi-rsp.c | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 5e03ed190e18..7567ca63aae2 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
struct ncsi_cmd_svf_pkt *cmd;
cmd = skb_put_zero(skb, sizeof(*cmd));
- cmd->vlan = htons(nca->words[0]);
- cmd->index = nca->bytes[2];
- cmd->enable = nca->bytes[3];
+ cmd->vlan = htons(nca->words[1]);
+ cmd->index = nca->bytes[6];
+ cmd->enable = nca->bytes[7];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
struct ncsi_cmd_ev_pkt *cmd;
cmd = skb_put_zero(skb, sizeof(*cmd));
- cmd->mode = nca->bytes[0];
+ cmd->mode = nca->bytes[3];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -228,7 +228,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
{ NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
{ NCSI_PKT_CMD_GLS,0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SVF,4, ncsi_cmd_handler_svf },
+ { NCSI_PKT_CMD_SVF,8, ncsi_cmd_handler_svf },
{ NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
{ NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_SMA,8, ncsi_cmd_handler_sma },
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 3ea49ed0a935..91b4b66438df 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
unsigned char index; /* VLAN table index */
unsigned char enable;/* Enable or disable */
__be32 checksum; /* Checksum */
- unsigned char pad[14];
+ unsigned char pad[18];
};
/* Enable VLAN */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 087db775b3dc..c1a191d790e2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
/* Add or remove the VLAN filter */
if (!(cmd->enable & 0x1)) {
- ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index);
+ /* HW indexes from 1 */
+ ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - 1);
} else {
vlan = ntohs(cmd->vlan);
ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan);
--
2.14.0
[PATCH net-next v3 2/3] net/ncsi: Configure VLAN tag filter
Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
stack process new VLAN tags and configure the channel VLAN filter
appropriately.
Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
for each one, meaning the ncsi_dev_state_config_svf state must be
repeated. An internal list of VLAN tags is maintained, and compared
against the current channel's ncsi_channel_filter in order to keep track
within the state. VLAN filters are removed in a similar manner, with the
introduction of the ncsi_dev_state_config_clear_vids state. The maximum
number of VLAN tag filters is determined by the "Get Capabilities"
response from the channel.
Signed-off-by: Samuel Mendoza-Jonas
---
v3: - Add comment describing change to ncsi_find_filter()
- Catch NULL in clear_one_vid() from ncsi_get_filter()
- Simplify state changes when kicking updated channel
include/net/ncsi.h | 2 +
net/ncsi/internal.h| 11 ++
net/ncsi/ncsi-manage.c | 308 -
net/ncsi/ncsi-rsp.c| 9 +-
4 files changed, 326 insertions(+), 4 deletions(-)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 68680baac0fd..1f96af46df49 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -28,6 +28,8 @@ struct ncsi_dev {
};
#ifdef CONFIG_NET_NCSI
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid);
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid);
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd));
int ncsi_start_dev(struct ncsi_dev *nd);
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1308a56f2591..af3d636534ef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -180,6 +180,7 @@ struct ncsi_channel {
#define NCSI_CHANNEL_INACTIVE 1
#define NCSI_CHANNEL_ACTIVE2
#define NCSI_CHANNEL_INVISIBLE 3
+ boolreconfigure_needed;
spinlock_t lock; /* Protect filters etc */
struct ncsi_package *package;
struct ncsi_channel_version version;
@@ -235,6 +236,9 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp= 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_clear_vids,
+ ncsi_dev_state_config_svf,
+ ncsi_dev_state_config_ev,
ncsi_dev_state_config_sma,
ncsi_dev_state_config_ebf,
#if IS_ENABLED(CONFIG_IPV6)
@@ -253,6 +257,12 @@ enum {
ncsi_dev_state_suspend_done
};
+struct vlan_vid {
+ struct list_head list;
+ __be16 proto;
+ u16 vid;
+};
+
struct ncsi_dev_priv {
struct ncsi_dev ndev;/* Associated NCSI device */
unsigned intflags; /* NCSI device flags */
@@ -276,6 +286,7 @@ struct ncsi_dev_priv {
struct work_struct work;/* For channel management */
struct packet_type ptype; /* NCSI packet Rx handler */
struct list_headnode;/* Form NCSI device list */
+ struct list_headvlan_vids; /* List of active VLAN IDs */
};
struct ncsi_cmd_arg {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a3bd5fa8ad09..11904b3b702d 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -38,6 +38,25 @@ static inline int ncsi_filter_size(int table)
return sizes[table];
}
+u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
+{
+ struct ncsi_channel_filter *ncf;
+ int size;
+
+ ncf = nc->filters[table];
+ if (!ncf)
+ return NULL;
+
+ size = ncsi_filter_size(table);
+ if (size < 0)
+ return NULL;
+
+ return ncf->data + size * index;
+}
+
+/* Find the first active filter in a filter table that matches the given
+ * data parameter. If data is NULL, this returns the first active filter.
+ */
int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
{
struct ncsi_channel_filter *ncf;
@@ -58,7 +77,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, void
*data)
index = -1;
while ((index = find_next_bit(bitmap, ncf->total, index + 1))
< ncf->total) {
- if (!memcmp(ncf->data + size * index, data, size)) {
+ if (!data || !memcmp(ncf->data + size * index, data, size)) {
spin_unlock_irqrestore(&nc->lock, flags);
return index;
}
@@ -639,6 +658,95 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_functional;
}
+/* Check the VLAN filter bitmap for a set filter, and construct a
+ * "Set VLAN Filter - Disable" packet if found.
+ */
+static
[PATCH net-next v3 3/3] ftgmac100: Support NCSI VLAN filtering when available
Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the
NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available.
This allows the VLAN core to notify the NCSI driver when changes occur
so that the remote NCSI channel can be properly configured to filter on
the set VLAN tags.
Signed-off-by: Samuel Mendoza-Jonas
---
v2: Moved ftgmac100 change into same patch and reordered
drivers/net/ethernet/faraday/ftgmac100.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 34dae51effd4..05fe7123d5ae 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops =
{
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= ftgmac100_poll_controller,
#endif
+ .ndo_vlan_rx_add_vid= ncsi_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid,
};
static int ftgmac100_setup_mdio(struct net_device *netdev)
@@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX |
NETIF_F_HW_VLAN_CTAG_TX;
+ if (priv->use_ncsi)
+ netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+
/* AST2400 doesn't have working HW checksum generation */
if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
netdev->hw_features &= ~NETIF_F_HW_CSUM;
--
2.14.0
Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
On Mon, 2017-08-14 at 11:39 +0930, Joel Stanley wrote:
> On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
> wrote:
> > Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
> > stack process new VLAN tags and configure the channel VLAN filter
> > appropriately.
> > Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
> > for each one, meaning the ncsi_dev_state_config_svf state must be
> > repeated. An internal list of VLAN tags is maintained, and compared
> > against the current channel's ncsi_channel_filter in order to keep track
> > within the state. VLAN filters are removed in a similar manner, with the
> > introduction of the ncsi_dev_state_config_clear_vids state. The maximum
> > number of VLAN tag filters is determined by the "Get Capabilities"
> > response from the channel.
> >
> > Signed-off-by: Samuel Mendoza-Jonas
>
> I've given this some testing, but there are a few things I saw below
> that we should sort out.
>
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
> > return sizes[table];
> > }
> >
> > +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
> > +{
> > + struct ncsi_channel_filter *ncf;
> > + int size;
> > +
> > + ncf = nc->filters[table];
> > + if (!ncf)
> > + return NULL;
> > +
> > + size = ncsi_filter_size(table);
> > + if (size < 0)
> > + return NULL;
> > +
> > + return ncf->data + size * index;
> > +}
> > +
> > int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
> > {
> > struct ncsi_channel_filter *ncf;
> > @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table,
> > void *data)
> > index = -1;
> > while ((index = find_next_bit(bitmap, ncf->total, index + 1))
> >< ncf->total) {
> > - if (!memcmp(ncf->data + size * index, data, size)) {
> > + if (!data || !memcmp(ncf->data + size * index, data, size))
> > {
>
> Not clear why this check is required?
Right, this could use a comment. This is a small workaround to having a
way to finding an arbitrary active filter, below in clear_one_vid(). We
pass NULL as a way of saying "return any enabled filter".
>
> > spin_unlock_irqrestore(&nc->lock, flags);
> > return index;
> > }
> > @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv
> > *ndp)
> > nd->state = ncsi_dev_state_functional;
> > }
> >
> > +/* Check the VLAN filter bitmap for a set filter, and construct a
> > + * "Set VLAN Filter - Disable" packet if found.
> > + */
> > +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel
> > *nc,
> > +struct ncsi_cmd_arg *nca)
> > +{
> > + int index;
> > + u16 vid;
> > +
> > + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
> > + if (index < 0) {
> > + /* Filter table empty */
> > + return -1;
> > + }
> > +
> > + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
>
> You just added this function that returns a pointer to a u32. It's
> strange to see the only call site then throw half of it away.
The problem here is that the single ncsi_channel_filter struct is used to
represent several different filters, and the filter data is stored in a
u32 data[] type. We cast to u16 in clear_one_vid because we know it's a
VLAN tag (NCSI_FILTER_VLAN), although we probably should call
ncsi_filter_size() to be sure.
>
> Also, ncsi_get_filter can return NULL.
That is indeed a problem though!
>
> > + netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > + "ncsi: removed vlan tag %u at index %d\n",
> > + vid, index + 1);
> > + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
> > +
> > + nca->type = NCSI_PKT_CMD_SVF;
> > + nca->words[1] = vid;
> > + /* HW filter index starts at 1 */
> > + nca->bytes[6] = index + 1;
> > + nca->bytes[7] = 0x00;
> > + return 0;
> > +}
> > @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct
> > ncsi_dev_priv *ndp)
> >
[PATCH net-next v2 3/3] ftgmac100: Support NCSI VLAN filtering when available
Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the
NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available.
This allows the VLAN core to notify the NCSI driver when changes occur
so that the remote NCSI channel can be properly configured to filter on
the set VLAN tags.
Signed-off-by: Samuel Mendoza-Jonas
---
v2: Moved ftgmac100 change into same patch and reordered
drivers/net/ethernet/faraday/ftgmac100.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 34dae51effd4..05fe7123d5ae 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops =
{
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= ftgmac100_poll_controller,
#endif
+ .ndo_vlan_rx_add_vid= ncsi_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid,
};
static int ftgmac100_setup_mdio(struct net_device *netdev)
@@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX |
NETIF_F_HW_VLAN_CTAG_TX;
+ if (priv->use_ncsi)
+ netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+
/* AST2400 doesn't have working HW checksum generation */
if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
netdev->hw_features &= ~NETIF_F_HW_CSUM;
--
2.14.0
[PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
stack process new VLAN tags and configure the channel VLAN filter
appropriately.
Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
for each one, meaning the ncsi_dev_state_config_svf state must be
repeated. An internal list of VLAN tags is maintained, and compared
against the current channel's ncsi_channel_filter in order to keep track
within the state. VLAN filters are removed in a similar manner, with the
introduction of the ncsi_dev_state_config_clear_vids state. The maximum
number of VLAN tag filters is determined by the "Get Capabilities"
response from the channel.
Signed-off-by: Samuel Mendoza-Jonas
---
include/net/ncsi.h | 2 +
net/ncsi/internal.h| 11 ++
net/ncsi/ncsi-manage.c | 295 -
net/ncsi/ncsi-rsp.c| 9 +-
4 files changed, 313 insertions(+), 4 deletions(-)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 68680baac0fd..1f96af46df49 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -28,6 +28,8 @@ struct ncsi_dev {
};
#ifdef CONFIG_NET_NCSI
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid);
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid);
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd));
int ncsi_start_dev(struct ncsi_dev *nd);
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1308a56f2591..af3d636534ef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -180,6 +180,7 @@ struct ncsi_channel {
#define NCSI_CHANNEL_INACTIVE 1
#define NCSI_CHANNEL_ACTIVE2
#define NCSI_CHANNEL_INVISIBLE 3
+ boolreconfigure_needed;
spinlock_t lock; /* Protect filters etc */
struct ncsi_package *package;
struct ncsi_channel_version version;
@@ -235,6 +236,9 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp= 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_clear_vids,
+ ncsi_dev_state_config_svf,
+ ncsi_dev_state_config_ev,
ncsi_dev_state_config_sma,
ncsi_dev_state_config_ebf,
#if IS_ENABLED(CONFIG_IPV6)
@@ -253,6 +257,12 @@ enum {
ncsi_dev_state_suspend_done
};
+struct vlan_vid {
+ struct list_head list;
+ __be16 proto;
+ u16 vid;
+};
+
struct ncsi_dev_priv {
struct ncsi_dev ndev;/* Associated NCSI device */
unsigned intflags; /* NCSI device flags */
@@ -276,6 +286,7 @@ struct ncsi_dev_priv {
struct work_struct work;/* For channel management */
struct packet_type ptype; /* NCSI packet Rx handler */
struct list_headnode;/* Form NCSI device list */
+ struct list_headvlan_vids; /* List of active VLAN IDs */
};
struct ncsi_cmd_arg {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a3bd5fa8ad09..3cbd4328f142 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
return sizes[table];
}
+u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
+{
+ struct ncsi_channel_filter *ncf;
+ int size;
+
+ ncf = nc->filters[table];
+ if (!ncf)
+ return NULL;
+
+ size = ncsi_filter_size(table);
+ if (size < 0)
+ return NULL;
+
+ return ncf->data + size * index;
+}
+
int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
{
struct ncsi_channel_filter *ncf;
@@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, void
*data)
index = -1;
while ((index = find_next_bit(bitmap, ncf->total, index + 1))
< ncf->total) {
- if (!memcmp(ncf->data + size * index, data, size)) {
+ if (!data || !memcmp(ncf->data + size * index, data, size)) {
spin_unlock_irqrestore(&nc->lock, flags);
return index;
}
@@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_functional;
}
+/* Check the VLAN filter bitmap for a set filter, and construct a
+ * "Set VLAN Filter - Disable" packet if found.
+ */
+static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
+struct ncsi_cmd_arg *nca)
+{
+ int index;
+ u16 vid;
+
+ index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
+ if (index < 0) {
+ /* Filter table empty */
+ return -1;
+ }
+
+ vid = *(u16 *)ncsi_get_fi
[PATCH net-next v2 0/3] NCSI VLAN Filtering Support
This series (mainly patch 2) adds VLAN filtering to the NCSI implementation. A fair amount of code already exists in the NCSI stack for VLAN filtering but none of it is actually hooked up. This goes the final mile and fixes a few bugs in the existing code found along the way (patch 1). Patch 3 adds the appropriate flag and callbacks to the ftgmac100 driver to enable filtering as it's a large consumer of NCSI (and what I've been testing on). Samuel Mendoza-Jonas (3): net/ncsi: Fix several packet definitions net/ncsi: Configure VLAN tag filter ftgmac100: Support NCSI VLAN filtering when available drivers/net/ethernet/faraday/ftgmac100.c | 5 + include/net/ncsi.h | 2 + net/ncsi/internal.h | 11 ++ net/ncsi/ncsi-cmd.c | 10 +- net/ncsi/ncsi-manage.c | 295 ++- net/ncsi/ncsi-pkt.h | 2 +- net/ncsi/ncsi-rsp.c | 12 +- 7 files changed, 326 insertions(+), 11 deletions(-) -- 2.14.0
[PATCH net-next v2 1/3] net/ncsi: Fix several packet definitions
Signed-off-by: Samuel Mendoza-Jonas
---
v2: Rebased on latest net-next
net/ncsi/ncsi-cmd.c | 10 +-
net/ncsi/ncsi-pkt.h | 2 +-
net/ncsi/ncsi-rsp.c | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 5e03ed190e18..7567ca63aae2 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
struct ncsi_cmd_svf_pkt *cmd;
cmd = skb_put_zero(skb, sizeof(*cmd));
- cmd->vlan = htons(nca->words[0]);
- cmd->index = nca->bytes[2];
- cmd->enable = nca->bytes[3];
+ cmd->vlan = htons(nca->words[1]);
+ cmd->index = nca->bytes[6];
+ cmd->enable = nca->bytes[7];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
struct ncsi_cmd_ev_pkt *cmd;
cmd = skb_put_zero(skb, sizeof(*cmd));
- cmd->mode = nca->bytes[0];
+ cmd->mode = nca->bytes[3];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -228,7 +228,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
{ NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
{ NCSI_PKT_CMD_GLS,0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SVF,4, ncsi_cmd_handler_svf },
+ { NCSI_PKT_CMD_SVF,8, ncsi_cmd_handler_svf },
{ NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
{ NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_SMA,8, ncsi_cmd_handler_sma },
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 3ea49ed0a935..91b4b66438df 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
unsigned char index; /* VLAN table index */
unsigned char enable;/* Enable or disable */
__be32 checksum; /* Checksum */
- unsigned char pad[14];
+ unsigned char pad[18];
};
/* Enable VLAN */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 087db775b3dc..c1a191d790e2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
/* Add or remove the VLAN filter */
if (!(cmd->enable & 0x1)) {
- ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index);
+ /* HW indexes from 1 */
+ ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - 1);
} else {
vlan = ntohs(cmd->vlan);
ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan);
--
2.14.0
Re: [PATCH 0/3] NCSI VLAN Filtering Support
On Fri, 2017-08-11 at 14:48 -0700, David Miller wrote: > From: Samuel Mendoza-Jonas > Date: Fri, 11 Aug 2017 16:16:45 +1000 > > > This series (mainly patch 3) adds VLAN filtering to the NCSI implementation. > > A fair amount of code already exists in the NCSI stack for VLAN filtering > > but > > none of it is actually hooked up. This goes the final mile and fixes a few > > bugs in the existing code found along the way (patch 2). > > > > Patch 1 adds the appropriate flag to the ftgmac100 driver to enable > > filtering > > as it's a large consumer of NCSI (and what I've been testing on). > > This patch series does not apply cleanly to net-next, you need to respin. Sorry about that, silly Friday mistake :) I noticed I've also squashed an ftgmac100 change into the wrong patch, I'll fix that up along with the respin. Cheers, Sam
[PATCH 1/3] ftgmac: Include NETIF_F_HW_VLAN_CTAG_FILTER in features
This is required for the VLAN core to call the add/kill callback for VLAN IDs. 'ftgmac100' already supports VLAN tagging but this flag lets the network stack know that we want to be notified of VLAN tags being added or removed when we have NCSI support. Signed-off-by: Samuel Mendoza-Jonas --- drivers/net/ethernet/faraday/ftgmac100.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 33b5c8eb9961..7c4d772287c7 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -1837,6 +1837,9 @@ static int ftgmac100_probe(struct platform_device *pdev) NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX; + if (priv->use_ncsi) + netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER; + /* AST2400 doesn't have working HW checksum generation */ if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac"))) netdev->hw_features &= ~NETIF_F_HW_CSUM; -- 2.14.0
[PATCH 2/3] net/ncsi: Fix several packet definitions
Signed-off-by: Samuel Mendoza-Jonas
---
net/ncsi/ncsi-cmd.c | 10 +-
net/ncsi/ncsi-pkt.h | 2 +-
net/ncsi/ncsi-rsp.c | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index db7083bfd476..1fec9fda7f60 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -146,9 +146,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
cmd = (struct ncsi_cmd_svf_pkt *)skb_put(skb, sizeof(*cmd));
memset(cmd, 0, sizeof(*cmd));
- cmd->vlan = htons(nca->words[0]);
- cmd->index = nca->bytes[2];
- cmd->enable = nca->bytes[3];
+ cmd->vlan = htons(nca->words[1]);
+ cmd->index = nca->bytes[6];
+ cmd->enable = nca->bytes[7];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -161,7 +161,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
cmd = (struct ncsi_cmd_ev_pkt *)skb_put(skb, sizeof(*cmd));
memset(cmd, 0, sizeof(*cmd));
- cmd->mode = nca->bytes[0];
+ cmd->mode = nca->bytes[3];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -240,7 +240,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
{ NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
{ NCSI_PKT_CMD_GLS,0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SVF,4, ncsi_cmd_handler_svf },
+ { NCSI_PKT_CMD_SVF,8, ncsi_cmd_handler_svf },
{ NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
{ NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_SMA,8, ncsi_cmd_handler_sma },
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 3ea49ed0a935..91b4b66438df 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
unsigned char index; /* VLAN table index */
unsigned char enable;/* Enable or disable */
__be32 checksum; /* Checksum */
- unsigned char pad[14];
+ unsigned char pad[18];
};
/* Enable VLAN */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 087db775b3dc..c1a191d790e2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
/* Add or remove the VLAN filter */
if (!(cmd->enable & 0x1)) {
- ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index);
+ /* HW indexes from 1 */
+ ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - 1);
} else {
vlan = ntohs(cmd->vlan);
ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan);
--
2.14.0
[PATCH 0/3] NCSI VLAN Filtering Support
This series (mainly patch 3) adds VLAN filtering to the NCSI implementation. A fair amount of code already exists in the NCSI stack for VLAN filtering but none of it is actually hooked up. This goes the final mile and fixes a few bugs in the existing code found along the way (patch 2). Patch 1 adds the appropriate flag to the ftgmac100 driver to enable filtering as it's a large consumer of NCSI (and what I've been testing on). Samuel Mendoza-Jonas (3): ftgmac: Include NETIF_F_HW_VLAN_CTAG_FILTER in features net/ncsi: Fix several packet definitions net/ncsi: Configure VLAN tag filter drivers/net/ethernet/faraday/ftgmac100.c | 5 + include/net/ncsi.h | 2 + net/ncsi/internal.h | 11 ++ net/ncsi/ncsi-cmd.c | 10 +- net/ncsi/ncsi-manage.c | 295 ++- net/ncsi/ncsi-pkt.h | 2 +- net/ncsi/ncsi-rsp.c | 12 +- 7 files changed, 326 insertions(+), 11 deletions(-) -- 2.14.0
[PATCH 3/3] net/ncsi: Configure VLAN tag filter
Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
stack process new VLAN tags and configure the channel VLAN filter
appropriately.
Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
for each one, meaning the ncsi_dev_state_config_svf state must be
repeated. An internal list of VLAN tags is maintained, and compared
against the current channel's ncsi_channel_filter in order to keep track
within the state. VLAN filters are removed in a similar manner, with the
introduction of the ncsi_dev_state_config_clear_vids state. The maximum
number of VLAN tag filters is determined by the "Get Capabilities"
response from the channel.
Signed-off-by: Samuel Mendoza-Jonas
---
drivers/net/ethernet/faraday/ftgmac100.c | 2 +
include/net/ncsi.h | 2 +
net/ncsi/internal.h | 11 ++
net/ncsi/ncsi-manage.c | 295 ++-
net/ncsi/ncsi-rsp.c | 9 +-
5 files changed, 315 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 7c4d772287c7..5a3661317026 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops =
{
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= ftgmac100_poll_controller,
#endif
+ .ndo_vlan_rx_add_vid= ncsi_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid,
};
static int ftgmac100_setup_mdio(struct net_device *netdev)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 68680baac0fd..1f96af46df49 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -28,6 +28,8 @@ struct ncsi_dev {
};
#ifdef CONFIG_NET_NCSI
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid);
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid);
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd));
int ncsi_start_dev(struct ncsi_dev *nd);
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1308a56f2591..af3d636534ef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -180,6 +180,7 @@ struct ncsi_channel {
#define NCSI_CHANNEL_INACTIVE 1
#define NCSI_CHANNEL_ACTIVE2
#define NCSI_CHANNEL_INVISIBLE 3
+ boolreconfigure_needed;
spinlock_t lock; /* Protect filters etc */
struct ncsi_package *package;
struct ncsi_channel_version version;
@@ -235,6 +236,9 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp= 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_clear_vids,
+ ncsi_dev_state_config_svf,
+ ncsi_dev_state_config_ev,
ncsi_dev_state_config_sma,
ncsi_dev_state_config_ebf,
#if IS_ENABLED(CONFIG_IPV6)
@@ -253,6 +257,12 @@ enum {
ncsi_dev_state_suspend_done
};
+struct vlan_vid {
+ struct list_head list;
+ __be16 proto;
+ u16 vid;
+};
+
struct ncsi_dev_priv {
struct ncsi_dev ndev;/* Associated NCSI device */
unsigned intflags; /* NCSI device flags */
@@ -276,6 +286,7 @@ struct ncsi_dev_priv {
struct work_struct work;/* For channel management */
struct packet_type ptype; /* NCSI packet Rx handler */
struct list_headnode;/* Form NCSI device list */
+ struct list_headvlan_vids; /* List of active VLAN IDs */
};
struct ncsi_cmd_arg {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a3bd5fa8ad09..3cbd4328f142 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
return sizes[table];
}
+u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
+{
+ struct ncsi_channel_filter *ncf;
+ int size;
+
+ ncf = nc->filters[table];
+ if (!ncf)
+ return NULL;
+
+ size = ncsi_filter_size(table);
+ if (size < 0)
+ return NULL;
+
+ return ncf->data + size * index;
+}
+
int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
{
struct ncsi_channel_filter *ncf;
@@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, void
*data)
index = -1;
while ((index = find_next_bit(bitmap, ncf->total, index + 1))
< ncf->total) {
- if (!memcmp(ncf->data + size * index, data, size)) {
+ if (!data || !memcmp(ncf->data + size * index, data, size)) {
spin_unlock_irqrestore(&n
[GIT] [4.13] NFC update
Hi David,
This is the NFC pull request for 4.13. We have:
- A conversion to unified device and GPIO APIs for the
fdp, pn544, and st{21,-nci} drivers.
- A fix for NFC device IDs allocation.
- A fix for the nfcmrvl driver firmware download mechanism.
- A trf7970a DT and GPIO cleanup and clock setting fix.
- A few fixes for potential overflows in the digital and LLCP code.
The following changes since commit 06d4d450db770a70b29fa0244d50390c85e7e3c7:
net: dsa: Fix legacy probing (2017-06-17 22:59:45 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git
tags/nfc-next-4.13-1
for you to fetch changes up to bd751808f9ff5e1822c627f6c4283009e66b2e53:
NFC: trf7970a: Correct register settings for 27MHz clock (2017-06-28 09:16:54
+0200)
Andy Shevchenko (13):
NFC: pn544: Switch to devm_acpi_dev_add_driver_gpios()
NFC: st21nfca: Add GPIO ACPI mapping table
NFC: st21nfca: Get rid of code duplication in ->probe()
NFC: fdp: Convert I2C driver to ->probe_new()
NFC: fdp: Convert to use devres API
NFC: fdp: Add GPIO ACPI mapping table
NFC: st-nci: Get rid of platform data
NFC: st-nci: Get rid of "interesting" use of interrupt polarity
NFC: st-nci: Covert to use GPIO descriptor
NFC: st-nci: Use unified device properties API meaningfully
NFC: st-nci: Add GPIO ACPI mapping table
NFC: st-nci: Get rid of code duplication in ->probe()
MAINTAINERS: Remove non-existing NFC platform data files
Colin Ian King (1):
NFC: trf7970a: fix check of clock frequencies, use && instead of ||
Geoff Lansberry (1):
NFC: trf7970a: Correct register settings for 27MHz clock
Gustavo A. R. Silva (2):
nfc: nci: remove unnecessary null check
NFC: add NULL checks to avoid potential NULL pointer dereference
Johan Hovold (8):
NFC: fix broken device allocation
NFC: nfcmrvl_uart: add missing tty-device sanity check
NFC: nfcmrvl: do not use device-managed resources
NFC: nfcmrvl: use nfc-device for firmware download
NFC: nfcmrvl: fix firmware-management initialisation
NFC: nfcmrvl_uart: fix device-node leak during probe
NFC: nfcmrvl_usb: use interface as phy device
NFC: nfcmrvl: allow gpio 0 for reset signalling
Mark Greer (12):
MAINTAINERS: NFC: trf7970a: Add Mark Greer as maintainer
NFC: trf7970a: Don't de-assert EN2 unless it was asserted
NFC: trf7970a: Fix inaccurate comment in trf7970a_probe()
NFC: trf7970a: Only check 'en2-rf-quirk' if EN2 is specified
NFC: trf7970a: Remove useless comment
NFC: trf7970a: Remove support for 'vin-voltage-override' DT property
NFC: trf7970a: Enable pins are active high not active low
NFC: trf7970a: Convert to descriptor based GPIO interface
NFC: trf7970a: Clean up coding style issues
NFC: digital: NFC-A SEL_RES must be one byte
NFC: digital: NFC-DEP Target WT(nfcdep,max) is now 14
Revert "NFC: trf7970a: Handle extra byte in response to Type 5 RMB
commands"
Markus Elfring (2):
NFC: digital: Improve a size determination in four functions
NFC: digital: Delete an error message for memory allocation failure
Mateusz Jurczyk (3):
nfc: Fix the sockaddr length sanitization in llcp_sock_connect
nfc: Ensure presence of required attributes in the activate_target handler
NFC: Add sockaddr length checks before accessing sa_family in bind
handlers
.../devicetree/bindings/net/nfc/trf7970a.txt | 10 +-
MAINTAINERS| 11 +-
drivers/nfc/Kconfig| 2 +-
drivers/nfc/fdp/fdp.c | 15 +-
drivers/nfc/fdp/i2c.c | 38 +-
drivers/nfc/nfcmrvl/fw_dnld.c | 7 +-
drivers/nfc/nfcmrvl/main.c | 40 ++-
drivers/nfc/nfcmrvl/uart.c | 11 +-
drivers/nfc/nfcmrvl/usb.c | 4 +-
drivers/nfc/nfcsim.c | 6 +-
drivers/nfc/pn544/i2c.c| 3 +-
drivers/nfc/st-nci/i2c.c | 164 ++---
drivers/nfc/st-nci/spi.c | 162 ++---
drivers/nfc/st21nfca/i2c.c | 62 +---
drivers/nfc/trf7970a.c | 391 ++---
include/linux/platform_data/nfcmrvl.h | 2 +-
include/linux/platform_data/st-nci.h | 31 --
net/nfc/core.c | 31 +-
net/nfc/digital_core.c | 12 +-
net/nfc/digital_dep.c | 2 +-
net/nfc/digital_technology.c | 3 +-
net/nfc/llcp_sock.c| 9 +-
net/nfc/nci/core.c
Re: [PATCH] nfc: Add sockaddr length checks before accessing sa_family in bind handlers
On Tue, Jun 13, 2017 at 06:44:28PM +0200, Mateusz Jurczyk wrote: > Verify that the caller-provided sockaddr structure is large enough to > contain the sa_family field, before accessing it in bind() handlers of the > AF_NFC socket. Since the syscall doesn't enforce a minimum size of the > corresponding memory region, very short sockaddrs (zero or one byte long) > result in operating on uninitialized memory while referencing .sa_family. > > Signed-off-by: Mateusz Jurczyk > --- > net/nfc/llcp_sock.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) Applied to nfc-next, thanks. Cheers, Samuel.
Re: [PATCH] nfc: nci: remove unnecessary null check
Hi Gustavo, On Tue, Jun 13, 2017 at 11:37:18AM -0500, Gustavo A. R. Silva wrote: > Remove unnecessary NULL check for pointer conn_info. > conn_info is set in list_for_each_entry() using container_of(), > which is never NULL. > > Addresses-Coverity-ID: 1362349 > Cc: Guenter Roeck > Signed-off-by: Gustavo A. R. Silva > --- > net/nfc/nci/core.c | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) Applied to nfc-next, thanks. Cheers, Samuel.
Re: [PATCH] nfc: Ensure presence of required attributes in the activate_target netlink handler
Hi Mateusz, On Wed, May 24, 2017 at 12:42:26PM +0200, Mateusz Jurczyk wrote: > Check that the NFC_ATTR_TARGET_INDEX and NFC_ATTR_PROTOCOLS attributes (in > addition to NFC_ATTR_DEVICE_INDEX) are provided by the netlink client > prior to accessing them. This prevents potential unhandled NULL pointer > dereference exceptions which can be triggered by malicious user-mode > programs, if they omit one or both of these attributes. > > Signed-off-by: Mateusz Jurczyk > --- > net/nfc/netlink.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Applied, thanks. Cheers, Samuel.
Re: [PATCH] nfc: Fix the sockaddr length sanitization in llcp_sock_connect
Hi Mateusz, On Wed, May 24, 2017 at 12:26:20PM +0200, Mateusz Jurczyk wrote: > Fix the sockaddr length verification in the connect() handler of NFC/LLCP > sockets, to compare against the size of the actual structure expected on > input (sockaddr_nfc_llcp) instead of its shorter version (sockaddr_nfc). > > Both structures are defined in include/uapi/linux/nfc.h. The fields > specific to the _llcp extended struct are as follows: > >276__u8 dsap; /* Destination SAP, if known */ >277__u8 ssap; /* Source SAP to be bound to */ >278char service_name[NFC_LLCP_MAX_SERVICE_NAME]; /* > Service name URI */; >279size_t service_name_len; > > If the caller doesn't provide a sufficiently long sockaddr buffer, these > fields remain uninitialized (and they currently originate from the stack > frame of the top-level sys_connect handler). They are then copied by > llcp_sock_connect() into internal storage (nfc_llcp_sock structure), and > could be subsequently read back through the user-mode getsockname() > function (handled by llcp_sock_getname()). This would result in the > disclosure of up to ~70 uninitialized bytes from the kernel stack to > user-mode clients capable of creating AFC_NFC sockets. > > Signed-off-by: Mateusz Jurczyk > --- > net/nfc/llcp_sock.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Applied to nfc-next, thanks. Cheers, Samuel.
Re: [PATCH 0/2] NFC-digital: Adjustments for four function implementations
Hi Markus, On Mon, May 22, 2017 at 02:57:42PM +0200, SF Markus Elfring wrote: > From: Markus Elfring > Date: Mon, 22 May 2017 14:50:05 +0200 > > Two update suggestions were taken into account > from static source code analysis. > > Markus Elfring (2): > Improve a size determination in four functions > Delete an error message for a failed memory allocation in digital_in_send() Both patches applied to nfc-next, thanks. Cheers, Samuel.
Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN
Arnd Bergmann, on ven. 16 juin 2017 17:41:47 +0200, wrote:
> The problem are the 'ch' and 'flag' variables that are passed into
> tty_insert_flip_char by value, and from there into
> tty_insert_flip_string_flags by reference. In this case, kasan tries
> to detect whether tty_insert_flip_string_flags() does any out-of-bounds
> access on the pointers and adds 64 bytes redzone around each of
> the two variables.
Ouch.
> gcc-6.3.1 happens to inline 16 calls of tty_insert_flip_char() into
> kbd_keycode(), so the stack size grows from 168 bytes to
> 168+(16*2*64) = 2216 bytes. There are 10 calls to put_queue()
> in to_utf8(), 12 in emulate_raw() and another 4 in kbd_keycode()
> itself.
That's why I agreed for put_queue :)
I'm however afraid we'd have to mark a lot of static functions that way,
depending on the aggressivity of gcc... I'd indeed really argue that gcc
should consider stack usage when inlining.
static int f(int foo) {
char c[256];
g(c, foo);
}
is really not something that I'd want to see the compiler to inline.
> > And no, we shouldn't need to do this. It sounds like ksan is the
> > problem here...
>
> Of course kasan is the problem, but it really just does whatever we
> asked it to do, and cannot do any better as long as we inline many
> copies of tty_insert_flip_char() into kbd_keycode().
We didn't ask to inline put_queue into kbd_keycode.
Samuel
Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN
Arnd Bergmann, on mer. 14 juin 2017 23:56:39 +0200, wrote: > > I however agree that it's a bad idea to inline it in functions where > > it's called so many times (and we're talking about the keyboard anyway). > > > >> -static void puts_queue(struct vc_data *vc, char *cp) > >> +static noinline_if_stackbloat void puts_queue(struct vc_data *vc, char > >> *cp) > > > > I don't see why, it's only called once in the callers. k_fn, however, is > > called several times in k_pad, so that could be why, but then it's > > rather be the inlining of k_fn which is a bad idea. > > It's called by applkey, which in turn is called by k_pad(), k_pad calls applkey twice only. Is that really to be considered bloat? > >> -static void fn_send_intr(struct vc_data *vc) > >> +static noinline_if_stackbloat void fn_send_intr(struct vc_data *vc) > > > > This one is only referenced, not called, I don't see how that could pose > > problem. > > I was surprised by this as well, but it seems that gcc these days is > smart enough to turn the indirect function calls for k_handler[type] > and/or f_handler[value] into inlines again when it has already > determined the index to be constant. Cool :) But I don't see how it can see find it out constant. The only fn_handler[] caller is k_spec, using value as index. The only caller of f_handler[] is kbd_keycode, using type as index, and keysym&0xff as value. That is definitely not constant :) And it's only one caller, I don't see how that can bloat. Samuel
Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN
Hello,
Arnd Bergmann, on mer. 14 juin 2017 23:15:38 +0200, wrote:
> As reported by kernelci, some functions in the VT code use significant
> amounts of kernel stack when local variables get inlined into the caller
> multiple times:
>
> drivers/tty/vt/keyboard.c: In function 'kbd_keycode':
> drivers/tty/vt/keyboard.c:1452:1: error: the frame size of 2240 bytes is
> larger than 2048 bytes [-Werror=frame-larger-than=]
>
> Annotating those functions as noinline_if_stackbloat prevents the inlining
> and reduces the overall stack usage in this driver.
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -301,13 +301,13 @@ int kbd_rate(struct kbd_repeat *rpt)
> /*
> * Helper Functions.
> */
> -static void put_queue(struct vc_data *vc, int ch)
> +static noinline_if_stackbloat void put_queue(struct vc_data *vc, int ch)
> {
> tty_insert_flip_char(&vc->port, ch, 0);
> tty_schedule_flip(&vc->port);
> }
I'm surprised that this be able generate so much stack use: the
tty_insert_flip_char inline only uses a pointer and an int.
And I'm surprised that multiple inlines can accumulate stack usage.
I however agree that it's a bad idea to inline it in functions where
it's called so many times (and we're talking about the keyboard anyway).
> -static void puts_queue(struct vc_data *vc, char *cp)
> +static noinline_if_stackbloat void puts_queue(struct vc_data *vc, char *cp)
I don't see why, it's only called once in the callers. k_fn, however, is
called several times in k_pad, so that could be why, but then it's
rather be the inlining of k_fn which is a bad idea.
> -static void fn_send_intr(struct vc_data *vc)
> +static noinline_if_stackbloat void fn_send_intr(struct vc_data *vc)
This one is only referenced, not called, I don't see how that could pose
problem.
Samuel
Re: [PATCH v2 0/8] NFC: fix device allocation and nfcmrvl crashes
Hi Johan, On Thu, Mar 30, 2017 at 12:15:34PM +0200, Johan Hovold wrote: > This started out with the observation that the nfcmrvl_uart driver > unconditionally dereferenced the tty class device despite the fact that > not every tty has an associated struct device (Unix98 ptys). Some > further changes were needed in the common nfcmrvl code to fully address > this, some of which also incidentally fixed a few related bugs (e.g. > resource leaks in error paths). > > While fixing this I stumbled over a regression in NFC core that lead to > broken registration error paths and misnamed workqueues. > > Note that this has only been tested by configuring the n_hci line > discipline for different ttys without any actual NFC hardware connected. > > Johan > > > Changes in v2 > - fix typo in commit message (1/8) > - release reset gpio in error paths (3/8) > - fix description of patch impact (3/8) > - allow gpio 0 to be used for reset signalling (8/8, new) > > > Johan Hovold (8): > NFC: fix broken device allocation > NFC: nfcmrvl_uart: add missing tty-device sanity check > NFC: nfcmrvl: do not use device-managed resources > NFC: nfcmrvl: use nfc-device for firmware download > NFC: nfcmrvl: fix firmware-management initialisation > NFC: nfcmrvl_uart: fix device-node leak during probe > NFC: nfcmrvl_usb: use interface as phy device > NFC: nfcmrvl: allow gpio 0 for reset signalling Applied, thanks. Cheers, Samuel.
[GIT] [4.12] NFC update
Hi David, This is the NFC pull request for 4.12. We have: - Improvements for the pn533 command queue handling and device registration order. - Removal of platform data for the pn544 and st21nfca drivers. - Additional device tree options to support more trf7970a hardware options. - Support for Sony's RC-S380P through the port100 driver. - Removal of the obsolte nfcwilink driver. - Headers inclusion cleanups (miscdevice.h, unaligned.h) for many drivers. The following changes since commit eefe06e8ceea88f8397a8df0880ab5ca28dcada6: Merge branch 'bpf-prog-testing-framework' (2017-04-01 12:45:58 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sameo/nfc-next.git tags/nfc-next-4.12-1 for you to fetch changes up to 4ea206395d3aede32bab94a75ec573530486fa44: nfc: fix get_unaligned_...() misuses (2017-04-17 00:42:22 +0200) NFC 4.12 pull request This is the NFC pull request for 4.12. We have: - Improvements for the pn533 command queue handling and device registration order. - Removal of platform data for the pn544 and st21nfca drivers. - Additional device tree options to support more trf7970a hardware options. - Support for Sony's RC-S380P through the port100 driver. - Removal of the obsolte nfcwilink driver. - Headers inclusion cleanups (miscdevice.h, unaligned.h) for many drivers. Al Viro (1): nfc: fix get_unaligned_...() misuses Andrey Rusalin (3): NFC: pn533: change order of free_irq and dev unregistration NFC: pn533: improve cmd queue handling NFC: pn533: change order operations in dev registation Andy Shevchenko (12): NFC: pn544: Get rid of platform data NFC: pn544: Convert to use GPIO descriptor NFC: pn544: Convert to use devm_request_threaded_irq() NFC: pn544: Add GPIO ACPI mapping table NFC: pn544: Get rid of code duplication in ->probe() NFC: st21nfca: Fix obvious typo when check error code NFC: st21nfca: Get rid of platform data NFC: st21nfca: Get rid of "interesting" use of interrupt polarity NFC: st21nfca: Covert to use GPIO descriptor NFC: st21nfca: Use unified device property API meaningfully NFC: netlink: Use error code from nfc_activate_target() NFC: Add nfc_dbg() macro Christophe JAILLET (1): NFC: st21nfca: Fix potential memory leak Corentin Labbe (3): nfc: nxp-nci: Remove unneeded linux/miscdevice.h include nfc: pn544: Remove unneeded linux/miscdevice.h include nfc: st21nfca: Remove unneeded linux/miscdevice.h include Dan Carpenter (1): NFC: nfcmrvl: double free on error path Geliang Tang (1): NFC: nfcmrvl: drop duplicate header gpio.h Geoff Lansberry (2): NFC: trf7970a: add device tree option for 27MHz clock NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage Guan Ben (1): NFC: Make EN2 pin optional in the TRF7970A driver Guenter Roeck (1): NFC: nxp-nci: Include unaligned.h instead of access_ok.h Michał Mirosław (1): NFC: pn533: use constant off-stack buffer for sending acks Nicholas Mc Guire (1): nfc: nxp-nci: use msleep for long delays OGAWA Hirofumi (4): nfc: Add support RC-S380P to port100 nfc: Send same info for both of NFC_CMD_GET_DEVICE and NFC_EVENT_DEVICE_ADDED nfc: Fix RC-S380* needs zero-length packet nfc: Fix hangup of RC-S380* in port100_send_ack() Rob Herring (1): NFC: remove TI nfcwilink driver Samuel Ortiz (1): MAINTAINERS: Remove Lauro and Aloisio from the NFC maintainers list Sudip Mukherjee (1): nfc: fdp: fix NULL pointer dereference Tobias Klauser (1): NFC: nfcmrvl: Include unaligned.h instead of access_ok.h .../devicetree/bindings/net/nfc/trf7970a.txt | 8 +- MAINTAINERS| 2 - drivers/nfc/Kconfig| 11 - drivers/nfc/Makefile | 1 - drivers/nfc/fdp/i2c.c | 6 +- drivers/nfc/nfcmrvl/fw_dnld.c | 7 +- drivers/nfc/nfcmrvl/spi.c | 6 +- drivers/nfc/nfcwilink.c| 578 - drivers/nfc/nxp-nci/firmware.c | 2 +- drivers/nfc/nxp-nci/i2c.c | 7 +- drivers/nfc/pn533/i2c.c| 34 +- drivers/nfc/pn533/pn533.c | 82 +-- drivers/nfc/pn533/pn533.h | 1 + drivers/nfc/pn533/usb.c| 8 +- drivers/nfc/pn544/i2c.c| 221 ++-- drivers/nfc/port100.c | 44 +- drivers/nfc/st21nfca/core.c| 12 +- drivers/nfc/st21nfca/i2c.c |
Re: [PATCH v2 0/8] NFC: fix device allocation and nfcmrvl crashes
Hi Johan, On Tue, Apr 18, 2017 at 12:09:16PM +0200, Johan Hovold wrote: > On Thu, Mar 30, 2017 at 12:15:34PM +0200, Johan Hovold wrote: > > This started out with the observation that the nfcmrvl_uart driver > > unconditionally dereferenced the tty class device despite the fact that > > not every tty has an associated struct device (Unix98 ptys). Some > > further changes were needed in the common nfcmrvl code to fully address > > this, some of which also incidentally fixed a few related bugs (e.g. > > resource leaks in error paths). > > > > While fixing this I stumbled over a regression in NFC core that lead to > > broken registration error paths and misnamed workqueues. > > > > Note that this has only been tested by configuring the n_hci line > > discipline for different ttys without any actual NFC hardware connected. > > > Johan Hovold (8): > > NFC: fix broken device allocation > > NFC: nfcmrvl_uart: add missing tty-device sanity check > > NFC: nfcmrvl: do not use device-managed resources > > NFC: nfcmrvl: use nfc-device for firmware download > > NFC: nfcmrvl: fix firmware-management initialisation > > NFC: nfcmrvl_uart: fix device-node leak during probe > > NFC: nfcmrvl_usb: use interface as phy device > > NFC: nfcmrvl: allow gpio 0 for reset signalling > > Any chance of getting these into 4.12, Samuel? I have yours and Mark Greer patchset pending. I'll push them to nfc-next and see if I can send another pull request to Dave by the end of this week. Cheers, Samuel.
