* [PATCH net] net: ethernet: stmmac: Fix signedness bug in ipq806x_gmac_of_parse()
From: Dan Carpenter @ 2019-09-25 11:05 UTC (permalink / raw)
To: Giuseppe Cavallaro, Mathieu Olivari
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Maxime Coquelin,
netdev, linux-stm32, kernel-janitors
The "gmac->phy_mode" variable is an enum and in this context GCC will
treat it as an unsigned int so the error handling will never be
triggered.
Fixes: b1c17215d718 ("stmmac: add ipq806x glue layer")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
index 2c6d7c69c8f7..0d21082ceb93 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
@@ -191,7 +191,7 @@ static int ipq806x_gmac_of_parse(struct ipq806x_gmac *gmac)
struct device *dev = &gmac->pdev->dev;
gmac->phy_mode = of_get_phy_mode(dev->of_node);
- if (gmac->phy_mode < 0) {
+ if ((int)gmac->phy_mode < 0) {
dev_err(dev, "missing phy mode property\n");
return -EINVAL;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: nixge: Fix a signedness bug in nixge_probe()
From: Dan Carpenter @ 2019-09-25 11:05 UTC (permalink / raw)
To: David S. Miller, Moritz Fischer
Cc: Andrew Lunn, Alex Williams, Luis Chamberlain, netdev,
kernel-janitors
The "priv->phy_mode" is an enum and in this context GCC will treat it
as an unsigned int so it can never be less than zero.
Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/ni/nixge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 0b384f97d2fd..2761f3a3ae50 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -1347,7 +1347,7 @@ static int nixge_probe(struct platform_device *pdev)
}
priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
- if (priv->phy_mode < 0) {
+ if ((int)priv->phy_mode < 0) {
netdev_err(ndev, "not find \"phy-mode\" property\n");
err = -EINVAL;
goto unregister_mdio;
--
2.20.1
^ permalink raw reply related
* Re: Fwd: [PATCH] bonding/802.3ad: fix slave initialization states race
From: Aleksei Zakharov @ 2019-09-25 11:01 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, zhangsha (A)
In-Reply-To: <16538.1569371467@famine>
ср, 25 сент. 2019 г. в 03:31, Jay Vosburgh <jay.vosburgh@canonical.com>:
>
> Алексей Захаров wrote:
> [...]
> >Right after reboot one of the slaves hangs with actor port state 71
> >and partner port state 1.
> >It doesn't send lacpdu and seems to be broken.
> >Setting link down and up again fixes slave state.
> [...]
>
> I think I see what failed in the first patch, could you test the
> following patch? This one is for net-next, so you'd need to again swap
> slave_err / netdev_err for the Ubuntu 4.15 kernel.
>
I've tested new patch. It seems to work. I can't reproduce the bug
with this patch.
There are two types of messages when link becomes up:
First:
bond-san: EVENT 1 llu 4294895911 slave eth2
8021q: adding VLAN 0 to HW filter on device eth2
bond-san: link status definitely down for interface eth2, disabling it
mlx4_en: eth2: Link Up
bond-san: EVENT 4 llu 4294895911 slave eth2
bond-san: link status up for interface eth2, enabling it in 500 ms
bond-san: invalid new link 3 on slave eth2
bond-san: link status definitely up for interface eth2, 10000 Mbps full duplex
Second:
bond-san: EVENT 1 llu 4295147594 slave eth2
8021q: adding VLAN 0 to HW filter on device eth2
mlx4_en: eth2: Link Up
bond-san: EVENT 4 llu 4295147594 slave eth2
bond-san: link status up again after 0 ms for interface eth2
bond-san: link status definitely up for interface eth2, 10000 Mbps full duplex
These messages (especially "invalid new link") look a bit unclear from
sysadmin point of view.
But lacp seems to work fine, thank you!
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 931d9d935686..5e248588259a 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1617,6 +1617,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> if (bond->params.miimon) {
> if (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS) {
> if (bond->params.updelay) {
> +/*XXX*/slave_info(bond_dev, slave_dev, "BOND_LINK_BACK initial state\n");
> bond_set_slave_link_state(new_slave,
> BOND_LINK_BACK,
> BOND_SLAVE_NOTIFY_NOW);
> @@ -2086,8 +2087,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> ignore_updelay = !rcu_dereference(bond->curr_active_slave);
>
> bond_for_each_slave_rcu(bond, slave, iter) {
> - slave->new_link = BOND_LINK_NOCHANGE;
> - slave->link_new_state = slave->link;
> + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
>
> link_state = bond_check_dev_link(bond, slave->dev, 0);
>
> @@ -2096,8 +2096,6 @@ static int bond_miimon_inspect(struct bonding *bond)
> if (link_state)
> continue;
>
> - bond_propose_link_state(slave, BOND_LINK_FAIL);
> - commit++;
> slave->delay = bond->params.downdelay;
> if (slave->delay) {
> slave_info(bond->dev, slave->dev, "link status down for %sinterface, disabling it in %d ms\n",
> @@ -2106,6 +2104,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> (bond_is_active_slave(slave) ?
> "active " : "backup ") : "",
> bond->params.downdelay * bond->params.miimon);
> + slave->link = BOND_LINK_FAIL;
> }
> /*FALLTHRU*/
> case BOND_LINK_FAIL:
> @@ -2121,7 +2120,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> }
>
> if (slave->delay <= 0) {
> - slave->new_link = BOND_LINK_DOWN;
> + bond_propose_link_state(slave, BOND_LINK_DOWN);
> commit++;
> continue;
> }
> @@ -2133,15 +2132,13 @@ static int bond_miimon_inspect(struct bonding *bond)
> if (!link_state)
> continue;
>
> - bond_propose_link_state(slave, BOND_LINK_BACK);
> - commit++;
> slave->delay = bond->params.updelay;
> -
> if (slave->delay) {
> slave_info(bond->dev, slave->dev, "link status up, enabling it in %d ms\n",
> ignore_updelay ? 0 :
> bond->params.updelay *
> bond->params.miimon);
> + slave->link = BOND_LINK_BACK;
> }
> /*FALLTHRU*/
> case BOND_LINK_BACK:
> @@ -2158,7 +2155,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> slave->delay = 0;
>
> if (slave->delay <= 0) {
> - slave->new_link = BOND_LINK_UP;
> + bond_propose_link_state(slave, BOND_LINK_UP);
> commit++;
> ignore_updelay = false;
> continue;
> @@ -2196,7 +2193,7 @@ static void bond_miimon_commit(struct bonding *bond)
> struct slave *slave, *primary;
>
> bond_for_each_slave(bond, slave, iter) {
> - switch (slave->new_link) {
> + switch (slave->link_new_state) {
> case BOND_LINK_NOCHANGE:
> /* For 802.3ad mode, check current slave speed and
> * duplex again in case its port was disabled after
> @@ -2268,8 +2265,8 @@ static void bond_miimon_commit(struct bonding *bond)
>
> default:
> slave_err(bond->dev, slave->dev, "invalid new link %d on slave\n",
> - slave->new_link);
> - slave->new_link = BOND_LINK_NOCHANGE;
> + slave->link_new_state);
> + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
>
> continue;
> }
> @@ -2677,13 +2674,13 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> bond_for_each_slave_rcu(bond, slave, iter) {
> unsigned long trans_start = dev_trans_start(slave->dev);
>
> - slave->new_link = BOND_LINK_NOCHANGE;
> + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
>
> if (slave->link != BOND_LINK_UP) {
> if (bond_time_in_interval(bond, trans_start, 1) &&
> bond_time_in_interval(bond, slave->last_rx, 1)) {
>
> - slave->new_link = BOND_LINK_UP;
> + bond_propose_link_state(slave, BOND_LINK_UP);
> slave_state_changed = 1;
>
> /* primary_slave has no meaning in round-robin
> @@ -2708,7 +2705,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> if (!bond_time_in_interval(bond, trans_start, 2) ||
> !bond_time_in_interval(bond, slave->last_rx, 2)) {
>
> - slave->new_link = BOND_LINK_DOWN;
> + bond_propose_link_state(slave, BOND_LINK_DOWN);
> slave_state_changed = 1;
>
> if (slave->link_failure_count < UINT_MAX)
> @@ -2739,8 +2736,8 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> goto re_arm;
>
> bond_for_each_slave(bond, slave, iter) {
> - if (slave->new_link != BOND_LINK_NOCHANGE)
> - slave->link = slave->new_link;
> + if (slave->link_new_state != BOND_LINK_NOCHANGE)
> + slave->link = slave->link_new_state;
> }
>
> if (slave_state_changed) {
> @@ -2763,9 +2760,9 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
> }
>
> /* Called to inspect slaves for active-backup mode ARP monitor link state
> - * changes. Sets new_link in slaves to specify what action should take
> - * place for the slave. Returns 0 if no changes are found, >0 if changes
> - * to link states must be committed.
> + * changes. Sets proposed link state in slaves to specify what action
> + * should take place for the slave. Returns 0 if no changes are found, >0
> + * if changes to link states must be committed.
> *
> * Called with rcu_read_lock held.
> */
> @@ -2777,12 +2774,12 @@ static int bond_ab_arp_inspect(struct bonding *bond)
> int commit = 0;
>
> bond_for_each_slave_rcu(bond, slave, iter) {
> - slave->new_link = BOND_LINK_NOCHANGE;
> + bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
> last_rx = slave_last_rx(bond, slave);
>
> if (slave->link != BOND_LINK_UP) {
> if (bond_time_in_interval(bond, last_rx, 1)) {
> - slave->new_link = BOND_LINK_UP;
> + bond_propose_link_state(slave, BOND_LINK_UP);
> commit++;
> }
> continue;
> @@ -2810,7 +2807,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
> if (!bond_is_active_slave(slave) &&
> !rcu_access_pointer(bond->current_arp_slave) &&
> !bond_time_in_interval(bond, last_rx, 3)) {
> - slave->new_link = BOND_LINK_DOWN;
> + bond_propose_link_state(slave, BOND_LINK_DOWN);
> commit++;
> }
>
> @@ -2823,7 +2820,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
> if (bond_is_active_slave(slave) &&
> (!bond_time_in_interval(bond, trans_start, 2) ||
> !bond_time_in_interval(bond, last_rx, 2))) {
> - slave->new_link = BOND_LINK_DOWN;
> + bond_propose_link_state(slave, BOND_LINK_DOWN);
> commit++;
> }
> }
> @@ -2843,7 +2840,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
> struct slave *slave;
>
> bond_for_each_slave(bond, slave, iter) {
> - switch (slave->new_link) {
> + switch (slave->link_new_state) {
> case BOND_LINK_NOCHANGE:
> continue;
>
> @@ -2893,8 +2890,9 @@ static void bond_ab_arp_commit(struct bonding *bond)
> continue;
>
> default:
> - slave_err(bond->dev, slave->dev, "impossible: new_link %d on slave\n",
> - slave->new_link);
> + slave_err(bond->dev, slave->dev,
> + "impossible: link_new_state %d on slave\n",
> + slave->link_new_state);
> continue;
> }
>
> @@ -3133,6 +3131,7 @@ static int bond_slave_netdev_event(unsigned long event,
> * let link-monitoring (miimon) set it right when correct
> * speeds/duplex are available.
> */
> +/*XXX*/slave_info(bond_dev, slave_dev, "EVENT %lu llu %lu\n", event, slave->last_link_up);
> if (bond_update_speed_duplex(slave) &&
> BOND_MODE(bond) == BOND_MODE_8023AD) {
> if (slave->last_link_up)
> diff --git a/include/net/bonding.h b/include/net/bonding.h
> index f7fe45689142..d416af72404b 100644
> --- a/include/net/bonding.h
> +++ b/include/net/bonding.h
> @@ -159,7 +159,6 @@ struct slave {
> unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
> s8 link; /* one of BOND_LINK_XXXX */
> s8 link_new_state; /* one of BOND_LINK_XXXX */
> - s8 new_link;
> u8 backup:1, /* indicates backup slave. Value corresponds with
> BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
> inactive:1, /* indicates inactive slave */
> @@ -549,7 +548,7 @@ static inline void bond_propose_link_state(struct slave *slave, int state)
>
> static inline void bond_commit_link_state(struct slave *slave, bool notify)
> {
> - if (slave->link == slave->link_new_state)
> + if (slave->link_new_state == BOND_LINK_NOCHANGE)
> return;
>
> slave->link = slave->link_new_state;
>
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
--
Best Regards,
Aleksei Zakharov
System administrator
Selectel - hosting for professionals
tel: +7 (812) 677-80-36
www.selectel.com
^ permalink raw reply
* [PATCH net] of: mdio: Fix a signedness bug in of_phy_get_and_connect()
From: Dan Carpenter @ 2019-09-25 11:01 UTC (permalink / raw)
To: Andrew Lunn, Dongpo Li
Cc: Florian Fainelli, Heiner Kallweit, Rob Herring, Frank Rowand,
netdev, devicetree, linux-kernel, kernel-janitors
The "iface" variable is an enum and in this context GCC treats it as
an unsigned int so the error handling is never triggered.
Fixes: b78624125304 ("of_mdio: Abstract a general interface for phy connect")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/of/of_mdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 000b95787df1..bd6129db6417 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -362,7 +362,7 @@ struct phy_device *of_phy_get_and_connect(struct net_device *dev,
int ret;
iface = of_get_phy_mode(np);
- if (iface < 0)
+ if ((int)iface < 0)
return NULL;
if (of_phy_is_fixed_link(np)) {
ret = of_phy_register_fixed_link(np);
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: axienet: fix a signedness bug in probe
From: Dan Carpenter @ 2019-09-25 10:59 UTC (permalink / raw)
To: Radhey Shyam Pandey, Alvaro G. M
Cc: David S. Miller, Michal Simek, Russell King, netdev,
kernel-janitors
The "lp->phy_mode" is an enum but in this context GCC treats it as an
unsigned int so the error handling is never triggered.
Fixes: ee06b1728b95 ("net: axienet: add support for standard phy-mode binding")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 4fc627fb4d11..676006f32f91 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1762,7 +1762,7 @@ static int axienet_probe(struct platform_device *pdev)
}
} else {
lp->phy_mode = of_get_phy_mode(pdev->dev.of_node);
- if (lp->phy_mode < 0) {
+ if ((int)lp->phy_mode < 0) {
ret = -EINVAL;
goto free_netdev;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: stmmac: dwmac-meson8b: Fix signedness bug in probe
From: Dan Carpenter @ 2019-09-25 10:58 UTC (permalink / raw)
To: Giuseppe Cavallaro, Martin Blumenstingl
Cc: Alexandre Torgue, Jose Abreu, David S. Miller, Kevin Hilman,
Maxime Coquelin, netdev, linux-stm32, kernel-janitors
The "dwmac->phy_mode" is an enum and in this context GCC treats it as
an unsigned int so the error handling is never triggered.
Fixes: 566e82516253 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 9cda29e4b89d..306da8f6b7d5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -339,7 +339,7 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
dwmac->dev = &pdev->dev;
dwmac->phy_mode = of_get_phy_mode(pdev->dev.of_node);
- if (dwmac->phy_mode < 0) {
+ if ((int)dwmac->phy_mode < 0) {
dev_err(&pdev->dev, "missing phy-mode property\n");
ret = -EINVAL;
goto err_remove_config_dt;
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: socionext: Fix a signedness bug in ave_probe()
From: Dan Carpenter @ 2019-09-25 10:57 UTC (permalink / raw)
To: Kunihiko Hayashi; +Cc: David S. Miller, netdev, kernel-janitors
The "phy_mode" variable is an enum and in this context GCC treats it as
an unsigned int so the error handling is never triggered.
Fixes: 4c270b55a5af ("net: ethernet: socionext: add AVE ethernet driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/socionext/sni_ave.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/socionext/sni_ave.c b/drivers/net/ethernet/socionext/sni_ave.c
index 10d0c3e478ab..d047a53f34f2 100644
--- a/drivers/net/ethernet/socionext/sni_ave.c
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -1566,7 +1566,7 @@ static int ave_probe(struct platform_device *pdev)
np = dev->of_node;
phy_mode = of_get_phy_mode(np);
- if (phy_mode < 0) {
+ if ((int)phy_mode < 0) {
dev_err(dev, "phy-mode not found\n");
return -EINVAL;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: broadcom/bcmsysport: Fix signedness in bcm_sysport_probe()
From: Dan Carpenter @ 2019-09-25 10:56 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, bcm-kernel-feedback-list, netdev,
kernel-janitors
The "priv->phy_interface" variable is an enum and in this context GCC
will treat it as unsigned so the error handling will never be
triggered.
Fixes: 80105befdb4b ("net: systemport: add Broadcom SYSTEMPORT Ethernet MAC driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 7df887e4024c..a977a459bd20 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -2481,7 +2481,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
priv->phy_interface = of_get_phy_mode(dn);
/* Default to GMII interface mode */
- if (priv->phy_interface < 0)
+ if ((int)priv->phy_interface < 0)
priv->phy_interface = PHY_INTERFACE_MODE_GMII;
/* In the case of a fixed PHY, the DT node associated
--
2.20.1
^ permalink raw reply related
* [PATCH net] enetc: Fix a signedness bug in enetc_of_get_phy()
From: Dan Carpenter @ 2019-09-25 10:57 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: David S. Miller, netdev, kernel-janitors
The "priv->if_mode" is type phy_interface_t which is an enum. In this
context GCC will treat the enum as an unsigned int so this error
handling is never triggered.
Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 7d6513ff8507..b73421c3e25b 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -785,7 +785,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
}
priv->if_mode = of_get_phy_mode(np);
- if (priv->if_mode < 0) {
+ if ((int)priv->if_mode < 0) {
dev_err(priv->dev, "missing phy type\n");
of_node_put(priv->phy_node);
if (of_phy_is_fixed_link(np))
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: netsec: Fix signedness bug in netsec_probe()
From: Dan Carpenter @ 2019-09-25 10:56 UTC (permalink / raw)
To: Jassi Brar
Cc: Ilias Apalodimas, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song, netdev,
bpf, kernel-janitors
The "priv->phy_interface" variable is an enum and in this context GCC
will treat it as an unsigned int so the error handling is never
triggered.
Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/socionext/netsec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 1502fe8b0456..55db7fbd43cc 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -2007,7 +2007,7 @@ static int netsec_probe(struct platform_device *pdev)
NETIF_MSG_LINK | NETIF_MSG_PROBE;
priv->phy_interface = device_get_phy_mode(&pdev->dev);
- if (priv->phy_interface < 0) {
+ if ((int)priv->phy_interface < 0) {
dev_err(&pdev->dev, "missing required property 'phy-mode'\n");
ret = -ENODEV;
goto free_ndev;
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: hisilicon: Fix signedness bug in hix5hd2_dev_probe()
From: Dan Carpenter @ 2019-09-25 10:55 UTC (permalink / raw)
To: Yisen Zhuang, Zhangfei Gao
Cc: Salil Mehta, David S. Miller, netdev, kernel-janitors
The "priv->phy_mode" variable is an enum and in this context GCC will
treat it as unsigned to the error handling will never trigger.
Fixes: 57c5bc9ad7d7 ("net: hisilicon: add hix5hd2 mac driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index 95a6b0926170..c41b19c760f8 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -1194,7 +1194,7 @@ static int hix5hd2_dev_probe(struct platform_device *pdev)
goto err_free_mdio;
priv->phy_mode = of_get_phy_mode(node);
- if (priv->phy_mode < 0) {
+ if ((int)priv->phy_mode < 0) {
netdev_err(ndev, "not find phy-mode\n");
ret = -EINVAL;
goto err_mdiobus;
--
2.20.1
^ permalink raw reply related
* [PATCH net] cxgb4: Signedness bug in init_one()
From: Dan Carpenter @ 2019-09-25 10:54 UTC (permalink / raw)
To: Vishal Kulkarni, Ganesh Goudar; +Cc: David S. Miller, netdev, kernel-janitors
The "chip" variable is an enum, and it's treated as unsigned int by GCC
in this context so the error handling isn't triggered.
Fixes: e8d452923ae6 ("cxgb4: clean up init_one")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 71854a19cebe..38024877751c 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5701,7 +5701,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
whoami = t4_read_reg(adapter, PL_WHOAMI_A);
pci_read_config_word(pdev, PCI_DEVICE_ID, &device_id);
chip = t4_get_chip_type(adapter, CHELSIO_PCI_ID_VER(device_id));
- if (chip < 0) {
+ if ((int)chip < 0) {
dev_err(&pdev->dev, "Device %d is not supported\n", device_id);
err = chip;
goto out_free_adapter;
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: aquantia: Fix aq_vec_isr_legacy() return value
From: Dan Carpenter @ 2019-09-25 10:54 UTC (permalink / raw)
To: Igor Russkikh, David VomLehn; +Cc: David S. Miller, netdev, kernel-janitors
The irqreturn_t type is an enum or an unsigned int in GCC. That
creates to problems because it can't detect if the
self->aq_hw_ops->hw_irq_read() call fails and at the end the function
always returns IRQ_HANDLED.
drivers/net/ethernet/aquantia/atlantic/aq_vec.c:316 aq_vec_isr_legacy() warn: unsigned 'err' is never less than zero.
drivers/net/ethernet/aquantia/atlantic/aq_vec.c:329 aq_vec_isr_legacy() warn: always true condition '(err >= 0) => (0-u32max >= 0)'
Fixes: 970a2e9864b0 ("net: ethernet: aquantia: Vector operations")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_vec.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
index 28892b8acd0e..a95c263a45aa 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_vec.c
@@ -306,15 +306,13 @@ irqreturn_t aq_vec_isr_legacy(int irq, void *private)
{
struct aq_vec_s *self = private;
u64 irq_mask = 0U;
- irqreturn_t err = 0;
+ int err;
- if (!self) {
- err = -EINVAL;
- goto err_exit;
- }
+ if (!self)
+ return IRQ_NONE;
err = self->aq_hw_ops->hw_irq_read(self->aq_hw, &irq_mask);
if (err < 0)
- goto err_exit;
+ return IRQ_NONE;
if (irq_mask) {
self->aq_hw_ops->hw_irq_disable(self->aq_hw,
@@ -322,11 +320,10 @@ irqreturn_t aq_vec_isr_legacy(int irq, void *private)
napi_schedule(&self->napi);
} else {
self->aq_hw_ops->hw_irq_enable(self->aq_hw, 1U);
- err = IRQ_NONE;
+ return IRQ_NONE;
}
-err_exit:
- return err >= 0 ? IRQ_HANDLED : IRQ_NONE;
+ return IRQ_HANDLED;
}
cpumask_t *aq_vec_get_affinity_mask(struct aq_vec_s *self)
--
2.20.1
^ permalink raw reply related
* RE: [PATCH v3 0/2] net: stmmac: Enhanced addressing mode for DWMAC 4.10
From: Jose Abreu @ 2019-09-25 10:44 UTC (permalink / raw)
To: David Miller, thierry.reding@gmail.com
Cc: peppe.cavallaro@st.com, alexandre.torgue@st.com,
f.fainelli@gmail.com, jonathanh@nvidia.com, bbiswas@nvidia.com,
netdev@vger.kernel.org, linux-tegra@vger.kernel.org
In-Reply-To: <20190924.214508.1949579574079200671.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sep/24/2019, 20:45:08 (UTC+00:00)
> From: Thierry Reding <thierry.reding@gmail.com>
> Date: Fri, 20 Sep 2019 19:00:34 +0200
>
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The DWMAC 4.10 supports the same enhanced addressing mode as later
> > generations. Parse this capability from the hardware feature registers
> > and set the EAME (Enhanced Addressing Mode Enable) bit when necessary.
>
> This looks like an enhancement and/or optimization rather than a bug fix.
Agree.
> Also, you're now writing to the high 32-bits unconditionally, even when
> it will always be zero because of 32-bit addressing. That looks like
> a step backwards to me.
Don't agree. As per previous discussions and as per my IP knowledge, if
EAME is not enabled / not supported the register can still be written.
This is not fast path and will not impact any remaining operation. Can
you please explain what exactly is the concern about this ?
Anyway, this is an important feature for performance so I hope Thierry
re-submits this once -next opens and addressing the review comments.
---
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH net-next] tuntap: Fallback to automq on TUNSETSTEERINGEBPF prog negative return
From: Michael S. Tsirkin @ 2019-09-25 10:33 UTC (permalink / raw)
To: Matt Cover
Cc: davem, ast, daniel, kafai, songliubraving, yhs, Jason Wang,
Eric Dumazet, Stanislav Fomichev, Matthew Cover, mail, pabeni,
Nicolas Dichtel, wangli39, lifei.shirley, tglx, netdev,
linux-kernel, bpf
In-Reply-To: <CAGyo_hpCDPmNvTau50XxRVkq1C=Qn7E8cVkE=BZhhiNF6MjqZA@mail.gmail.com>
On Sun, Sep 22, 2019 at 03:46:19PM -0700, Matt Cover wrote:
> Unless of course we can simply state via
> documentation that any negative return
> for which a define doesn't exist is
> undefined behavior. In which case,
> there is no old vs new behavior and
> no need for an ioctl. Simply the
> understanding provided by the
> documentation.
Unfortunately this isn't sufficient: software can easily return a wrong
value by mistake, and become dependent on an undefined behaviour.
--
MST
^ permalink raw reply
* RE: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
From: Jianyong Wu (Arm Technology China) @ 2019-09-25 10:27 UTC (permalink / raw)
To: Paolo Bonzini, netdev@vger.kernel.org, yangbo.lu@nxp.com,
john.stultz@linaro.org, tglx@linutronix.de,
sean.j.christopherson@intel.com, maz@kernel.org,
richardcochran@gmail.com, Mark Rutland, Will Deacon,
Suzuki Poulose
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Steve Capper,
Kaly Xin (Arm Technology China), Justin He (Arm Technology China),
nd, linux-arm-kernel@lists.infradead.org
In-Reply-To: <06264d8a-b9c0-5f19-db2c-6190976a2a05@redhat.com>
Hi Paolo,
> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Tuesday, September 24, 2019 10:20 PM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>;
> netdev@vger.kernel.org; yangbo.lu@nxp.com; john.stultz@linaro.org;
> tglx@linutronix.de; sean.j.christopherson@intel.com; maz@kernel.org;
> richardcochran@gmail.com; Mark Rutland <Mark.Rutland@arm.com>; Will
> Deacon <Will.Deacon@arm.com>; Suzuki Poulose
> <Suzuki.Poulose@arm.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; Steve Capper
> <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>; nd <nd@arm.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [RFC PATCH v3 4/6] psci: Add hvc call service for ptp_kvm.
>
> On 23/09/19 06:57, Jianyong Wu (Arm Technology China) wrote:
> >> On 19/09/19 11:46, Jianyong Wu (Arm Technology China) wrote:
> >>>> On 18/09/19 11:57, Jianyong Wu (Arm Technology China) wrote:
> >>>>> Paolo Bonzini wrote:
> >>>>>> This is not Y2038-safe. Please use ktime_get_real_ts64 instead,
> >>>>>> and split the 64-bit seconds value between val[0] and val[1].
> >>>
> >>> Val[] should be long not u32 I think, so in arm64 I can avoid that
> >>> Y2038_safe, but also need rewrite for arm32.
> >>
> >> I don't think there's anything inherently wrong with u32 val[], and
> >> as you notice it lets you reuse code between arm and arm64. It's up
> >> to you and Marc to decide.
> >>
> > To compatible 32-bit, Integrates second value and nanosecond value as
> > a nanosecond value then split it into val[0] and val[1] and split cycle value
> into val[2] and val[3], In this way, time will overflow at Y2262.
> > WDYT?
>
> So if I understand correctly you'd multiply by 10^9 (or better shift by
> 30) the nanoseconds.
>
Yeah,
> That works, but why not provide 5 output registers? Alternatively, take an
> address as input and write there.
It will be easy, if I could have expanded the store room. But these code is the infrastructure for hypercall, I can't change them at my will.
I think only value but pointer can delivered by smccc_set_retval call.
>
> Finally, on x86 we added an argument for the CLOCK_* that is being read
> (currently only CLOCK_REALTIME, but having room for extensibility in the API
> is always nice).
>
IMO, I will be limited by the design of hypercall on arm64, I can only design my code under it. maybe it will be better sometime but for now I could just obey it.
Thanks
Jianyong Wu
> Paolo
^ permalink raw reply
* [PATCH] man: add note to ip-macsec manual about necessary key management
From: Thomas Haller @ 2019-09-25 10:24 UTC (permalink / raw)
To: netdev; +Cc: stephen, Thomas Haller
The man page of ip-macsec and the existance of the tool makes it seem like
the user could just configure static keys once, and be done with it. That is
not the case. Some form or key management must be done in user space.
Add a note about that.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
man/man8/ip-macsec.8 | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/man/man8/ip-macsec.8 b/man/man8/ip-macsec.8
index 4fd8a5b6591a..2179b33683d5 100644
--- a/man/man8/ip-macsec.8
+++ b/man/man8/ip-macsec.8
@@ -102,8 +102,19 @@ type.
.SS Display MACsec configuration
.nf
# ip macsec show
+
+.SH NOTES
+This tool can be used to configure the 802.1AE keys of the interface. Note that 802.1AE uses GCM-AES
+with a initialization vector (IV) derived from the packet number. The same key must not be used
+with the same IV more than once. Instead, keys must be frequently regenerated and distibuted.
+This tool is thus mostly for debugging and testing, or in combination with a user-space application
+that reconfigures the keys. It is wrong to just configure the keys statically and assume them to work
+indefinitely. The suggested and standardized way for key management is 802.1X-2010, which is implemented
+by wpa_supplicant.
+
.SH SEE ALSO
.br
.BR ip-link (8)
+.BR wpa_supplicant (8)
.SH AUTHOR
Sabrina Dubroca <sd@queasysnail.net>
--
2.21.0
^ permalink raw reply related
* Re: WireGuard to port to existing Crypto API
From: Jason A. Donenfeld @ 2019-09-25 10:14 UTC (permalink / raw)
To: David Miller; +Cc: WireGuard mailing list, Netdev, LKML
In-Reply-To: <20190925.113928.2046484827308019751.davem@davemloft.net>
Hi Dave,
On Wed, Sep 25, 2019 at 12:03 PM David Miller <davem@davemloft.net> wrote:
> I didn't say "must" anything, I suggested this as a more smoothe
> and efficient way forward.
s/must/should/g? However it's characterized, I think your jugements
and opinions are generally sound, and I intend to put them into
action.
> I'm also a bit disappointed that you felt the need to so quickly
> make such an explosive posting to the mailing list when we've
Explosive? That's certainly not the intent here. The project is
changing direction in a big way. Collaborating with others on the
crypto API will be an important part of that. Announcing the change in
direction, those intentions, a rationale on why it will be okay, and
inviting collaboration is a responsible thing to do at the earliest
opportunity. Better to announce intent early rather than surprise
people or deter potential collaborators by keeping plans secret.
Jason
^ permalink raw reply
* Re: macb: inconsistent Rx descriptor chain after OOM
From: Harini Katakam @ 2019-09-25 10:05 UTC (permalink / raw)
To: Claudiu Beznea; +Cc: schwab, Nicolas Ferre, netdev
In-Reply-To: <379c59d0-e31b-96c1-8a5e-416b98583da0@microchip.com>
Hi Andreas,
On Mon, Sep 16, 2019 at 4:25 PM <Claudiu.Beznea@microchip.com> wrote:
>
>
>
> On 16.09.2019 13:14, Andreas Schwab wrote:
> > External E-Mail
> >
> >
> > On Sep 16 2019, <Claudiu.Beznea@microchip.com> wrote:
> >
> >> I will have a look on it. It would be good if you could give me some
> >> details about the steps to reproduce it.
> >
> > You need to trigger OOM.
>
> Ok, thank you!
>
> >
> > Andreas.
Can you please try incrementing the rx_prepared_head after skb
allocation as follows?
@@ -920,7 +920,6 @@ static void gem_rx_refill(struct macb_queue *queue)
/* Make hw descriptor updates visible to CPU */
rmb();
- queue->rx_prepared_head++;
desc = macb_rx_desc(queue, entry);
if (!queue->rx_skbuff[entry]) {
@@ -959,6 +958,7 @@ static void gem_rx_refill(struct macb_queue *queue)
dma_wmb();
desc->addr &= ~MACB_BIT(RX_USED);
}
+ queue->rx_prepared_head++;
}
/* Make descriptor updates visible to hardware */
Without this, head will increase even when skb allocation fails. It is a valid
fix anyway and I'll patch it. But I recall a *similar* issue with inconsistent
RX BD chain that was solved by this.
Regards,
Harini
^ permalink raw reply
* Re: WireGuard to port to existing Crypto API
From: David Miller @ 2019-09-25 9:40 UTC (permalink / raw)
To: bruno; +Cc: Jason, wireguard, netdev, linux-kernel
In-Reply-To: <20190925091700.GA9970@wolff.to>
From: Bruno Wolff III <bruno@wolff.to>
Date: Wed, 25 Sep 2019 04:17:00 -0500
> Are there going to be two branches, one for using the current API and
> one using Zinc?
This is inapproprate to even discuss at this point.
Let's see what the crypto based stuff looks like, evaluate it,
and then decide how to proceed forward.
Thank you.
^ permalink raw reply
* Re: WireGuard to port to existing Crypto API
From: David Miller @ 2019-09-25 9:39 UTC (permalink / raw)
To: Jason; +Cc: wireguard, netdev, linux-kernel
In-Reply-To: <CAHmME9pmfZAp5zd9BDLFc2fWUhtzZcjYZc2atTPTyNFFmEdHLg@mail.gmail.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Wed, 25 Sep 2019 10:29:45 +0200
> His viewpoint has recently solidified: in order to go upstream,
> WireGuard must port to the existing crypto API, and handle the Zinc
> project separately.
I didn't say "must" anything, I suggested this as a more smoothe
and efficient way forward.
I'm also a bit disappointed that you felt the need to so quickly
make such an explosive posting to the mailing list when we've
just spoken about this amongst ourselves only 20 minutes ago.
Please proceed in a more smoothe and considerate manner for all
parties involved.
Thank you.
^ permalink raw reply
* [PATCH] bpf: clean up indentation issue
From: Colin King @ 2019-09-25 9:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, netdev, bpf
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a statement that is indented one level too deeply,
remove the extraneous tab.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
kernel/bpf/btf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 722d38e543e9..29c7c06c6bd6 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2332,7 +2332,7 @@ static int btf_enum_check_kflag_member(struct btf_verifier_env *env,
if (BITS_PER_BYTE_MASKED(struct_bits_off)) {
btf_verifier_log_member(env, struct_type, member,
"Member is not byte aligned");
- return -EINVAL;
+ return -EINVAL;
}
nr_bits = int_bitsize;
--
2.20.1
^ permalink raw reply related
* Re: WireGuard to port to existing Crypto API
From: Bruno Wolff III @ 2019-09-25 9:17 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: WireGuard mailing list, Netdev, LKML
In-Reply-To: <CAHmME9pmfZAp5zd9BDLFc2fWUhtzZcjYZc2atTPTyNFFmEdHLg@mail.gmail.com>
Are there going to be two branches, one for using the current API and one
using Zinc?
^ permalink raw reply
* RE: [PATCH V2 6/8] mdev: introduce virtio device and its device ops
From: Tian, Kevin @ 2019-09-25 9:09 UTC (permalink / raw)
To: Jason Wang, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
alex.williamson@redhat.com, mst@redhat.com, Bie, Tiwei
Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
cohuck@redhat.com, maxime.coquelin@redhat.com, Liang, Cunming,
Wang, Zhihong, rob.miller@broadcom.com, Wang, Xiao W,
haotian.wang@sifive.com, zhenyuw@linux.intel.com, Wang, Zhi A,
jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com,
Vivi, Rodrigo, airlied@linux.ie, daniel@ffwll.ch,
farman@linux.ibm.com, pasic@linux.ibm.com, sebott@linux.ibm.com,
oberpar@linux.ibm.com, heiko.carstens@de.ibm.com,
gor@linux.ibm.com, borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
freude@linux.ibm.com, Zhu, Lingshan, idos@mellanox.com,
eperezma@redhat.com, lulu@redhat.com, parav@mellanox.com,
christophe.de.dinechin@gmail.com
In-Reply-To: <20190924135332.14160-7-jasowang@redhat.com>
> From: Jason Wang [mailto:jasowang@redhat.com]
> Sent: Tuesday, September 24, 2019 9:54 PM
>
> This patch implements basic support for mdev driver that supports
> virtio transport for kernel virtio driver.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> include/linux/mdev.h | 2 +
> include/linux/virtio_mdev.h | 145
> ++++++++++++++++++++++++++++++++++++
> 2 files changed, 147 insertions(+)
> create mode 100644 include/linux/virtio_mdev.h
>
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 3414307311f1..73ac27b3b868 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -126,6 +126,8 @@ struct mdev_device *mdev_from_dev(struct device
> *dev);
>
> enum {
> MDEV_ID_VFIO = 1,
> + MDEV_ID_VIRTIO = 2,
> + MDEV_ID_VHOST = 3,
> /* New entries must be added here */
> };
>
> diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h
> new file mode 100644
> index 000000000000..d1a40a739266
> --- /dev/null
> +++ b/include/linux/virtio_mdev.h
> @@ -0,0 +1,145 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Virtio mediated device driver
> + *
> + * Copyright 2019, Red Hat Corp.
> + * Author: Jason Wang <jasowang@redhat.com>
> + */
> +#ifndef _LINUX_VIRTIO_MDEV_H
> +#define _LINUX_VIRTIO_MDEV_H
> +
> +#include <linux/interrupt.h>
> +#include <linux/mdev.h>
> +#include <uapi/linux/vhost.h>
> +
> +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev"
> +#define VIRTIO_MDEV_VERSION 0x1
Just be curious. is this version identical to virtio spec version that below
callbacks are created for, or just irrelevant?
> +
> +struct virtio_mdev_callback {
> + irqreturn_t (*callback)(void *data);
> + void *private;
> +};
> +
> +/**
> + * struct vfio_mdev_device_ops - Structure to be registered for each
> + * mdev device to register the device to virtio-mdev module.
> + *
> + * @set_vq_address: Set the address of virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * @desc_area: address of desc area
> + * @driver_area: address of driver area
> + * @device_area: address of device area
> + * Returns integer: success (0) or error (< 0)
> + * @set_vq_num: Set the size of virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * @num: the size of virtqueue
> + * @kick_vq: Kick the virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * @set_vq_cb: Set the interrut calback function for
> + * a virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * @cb: virtio-mdev interrupt callback
> structure
> + * @set_vq_ready: Set ready status for a virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * @ready: ready (true) not ready(false)
> + * @get_vq_ready: Get ready status for a virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * Returns boolean: ready (true) or not (false)
> + * @set_vq_state: Set the state for a virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * @state: virtqueue state (last_avail_idx)
> + * Returns integer: success (0) or error (< 0)
> + * @get_vq_state: Get the state for a virtqueue
> + * @mdev: mediated device
> + * @idx: virtqueue index
> + * Returns virtqueue state (last_avail_idx)
> + * @get_vq_align: Get the virtqueue align requirement
> + * for the device
> + * @mdev: mediated device
> + * Returns virtqueue algin requirement
> + * @get_features: Get virtio features supported by the device
> + * @mdev: mediated device
> + * Returns the features support by the
> + * device
> + * @get_features: Set virtio features supported by the driver
> + * @mdev: mediated device
> + * @features: feature support by the driver
> + * Returns integer: success (0) or error (< 0)
> + * @set_config_cb: Set the config interrupt callback
> + * @mdev: mediated device
> + * @cb: virtio-mdev interrupt callback
> structure
> + * @get_device_id: Get virtio device id
> + * @mdev: mediated device
> + * Returns u32: virtio device id
> + * @get_vendor_id: Get virtio vendor id
> + * @mdev: mediated device
> + * Returns u32: virtio vendor id
> + * @get_status: Get the device status
> + * @mdev: mediated device
> + * Returns u8: virtio device status
> + * @set_status: Set the device status
> + * @mdev: mediated device
> + * @status: virtio device status
> + * @get_config: Read from device specific confiugration
> space
configuration (and similar typos downward)
> + * @mdev: mediated device
> + * @offset: offset from the beginning of
> + * configuration space
> + * @buf: buffer used to read to
> + * @len: the length to read from
> + * configration space
> + * @set_config: Write to device specific confiugration space
> + * @mdev: mediated device
> + * @offset: offset from the beginning of
> + * configuration space
> + * @buf: buffer used to write from
> + * @len: the length to write to
> + * configration space
> + * @get_version: Get the version of virtio mdev device
> + * @mdev: mediated device
> + * Returns integer: version of the device
> + * @get_generation: Get device generaton
> + * @mdev: mediated device
> + * Returns u32: device generation
> + */
> +struct virtio_mdev_device_ops {
> + /* Virtqueue ops */
> + int (*set_vq_address)(struct mdev_device *mdev,
> + u16 idx, u64 desc_area, u64 driver_area,
> + u64 device_area);
> + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num);
> + void (*kick_vq)(struct mdev_device *mdev, u16 idx);
> + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx,
> + struct virtio_mdev_callback *cb);
> + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool
> ready);
> + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx);
> + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state);
> + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx);
> +
> + /* Device ops */
> + u16 (*get_vq_align)(struct mdev_device *mdev);
> + u64 (*get_features)(struct mdev_device *mdev);
> + int (*set_features)(struct mdev_device *mdev, u64 features);
> + void (*set_config_cb)(struct mdev_device *mdev,
> + struct virtio_mdev_callback *cb);
> + u16 (*get_queue_max)(struct mdev_device *mdev);
> + u32 (*get_device_id)(struct mdev_device *mdev);
> + u32 (*get_vendor_id)(struct mdev_device *mdev);
> + u8 (*get_status)(struct mdev_device *mdev);
> + void (*set_status)(struct mdev_device *mdev, u8 status);
> + void (*get_config)(struct mdev_device *mdev, unsigned int offset,
> + void *buf, unsigned int len);
> + void (*set_config)(struct mdev_device *mdev, unsigned int offset,
> + const void *buf, unsigned int len);
> + int (*get_version)(struct mdev_device *mdev);
> + u32 (*get_generation)(struct mdev_device *mdev);
> +};
I'm not sure how stable above ops are. Does it make sense if defining
just two callbacks here, e.g. vq_ctrl and device_ctrl, and then let the
vendor driver to handle specific ops in each category (similar to how
ioctl works)?
Thanks
Kevin
^ permalink raw reply
* [PATCH] can: peakcan: report bus recovery as well
From: Jeroen Hofstee @ 2019-09-25 8:58 UTC (permalink / raw)
To: linux-can@vger.kernel.org
Cc: Jeroen Hofstee, Stephane Grosjean, Wolfgang Grandegger,
Marc Kleine-Budde, David S. Miller, Gustavo A. R. Silva,
Nicolas Ferre, Greg Kroah-Hartman, Thomas Gleixner,
open list:NETWORKING DRIVERS, open list
While the state changes are reported when the error counters increase
and decrease, there is no event when the bus recovers and the error
counters decrease again. So add those as well.
Change the state going downward to be ERROR_PASSIVE -> ERROR_WARNING ->
ERROR_ACTIVE instead of directly to ERROR_ACTIVE again.
Signed-off-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Cc: Stephane Grosjean <s.grosjean@peak-system.com>
---
drivers/net/can/usb/peak_usb/pcan_usb.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 617da295b6c1..dd2a7f529012 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -436,8 +436,8 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
}
if ((n & PCAN_USB_ERROR_BUS_LIGHT) == 0) {
/* no error (back to active state) */
- mc->pdev->dev.can.state = CAN_STATE_ERROR_ACTIVE;
- return 0;
+ new_state = CAN_STATE_ERROR_ACTIVE;
+ break;
}
break;
@@ -460,9 +460,9 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
}
if ((n & PCAN_USB_ERROR_BUS_HEAVY) == 0) {
- /* no error (back to active state) */
- mc->pdev->dev.can.state = CAN_STATE_ERROR_ACTIVE;
- return 0;
+ /* no error (back to warning state) */
+ new_state = CAN_STATE_ERROR_WARNING;
+ break;
}
break;
@@ -501,6 +501,11 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
mc->pdev->dev.can.can_stats.error_warning++;
break;
+ case CAN_STATE_ERROR_ACTIVE:
+ cf->can_id |= CAN_ERR_CRTL;
+ cf->data[1] = CAN_ERR_CRTL_ACTIVE;
+ break;
+
default:
/* CAN_STATE_MAX (trick to handle other errors) */
cf->can_id |= CAN_ERR_CRTL;
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox