* [PATCH net-next 2/3] ice: add i2c write command
From: Tony Nguyen @ 2022-05-17 21:19 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet
Cc: Karol Kolacinski, netdev, anthony.l.nguyen, richardcochran,
Gurucharan
In-Reply-To: <20220517211935.1949447-1-anthony.l.nguyen@intel.com>
From: Karol Kolacinski <karol.kolacinski@intel.com>
Add the possibility to write to connected i2c devices using the AQ
command. FW may reject the write if the device is not on allowlist.
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 7 +--
drivers/net/ethernet/intel/ice/ice_common.c | 51 ++++++++++++++++++-
drivers/net/ethernet/intel/ice/ice_common.h | 4 ++
3 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index b25e27c4d887..bedc19f12cbd 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1401,7 +1401,7 @@ struct ice_aqc_get_link_topo {
u8 rsvd[9];
};
-/* Read I2C (direct, 0x06E2) */
+/* Read/Write I2C (direct, 0x06E2/0x06E3) */
struct ice_aqc_i2c {
struct ice_aqc_link_topo_addr topo_addr;
__le16 i2c_addr;
@@ -1411,7 +1411,7 @@ struct ice_aqc_i2c {
u8 rsvd;
__le16 i2c_bus_addr;
- u8 rsvd2[4];
+ u8 i2c_data[4]; /* Used only by write command, reserved in read. */
};
/* Read I2C Response (direct, 0x06E2) */
@@ -2130,7 +2130,7 @@ struct ice_aq_desc {
struct ice_aqc_get_link_status get_link_status;
struct ice_aqc_event_lan_overflow lan_overflow;
struct ice_aqc_get_link_topo get_link_topo;
- struct ice_aqc_i2c read_i2c;
+ struct ice_aqc_i2c read_write_i2c;
struct ice_aqc_read_i2c_resp read_i2c_resp;
} params;
};
@@ -2247,6 +2247,7 @@ enum ice_adminq_opc {
ice_aqc_opc_set_mac_lb = 0x0620,
ice_aqc_opc_get_link_topo = 0x06E0,
ice_aqc_opc_read_i2c = 0x06E2,
+ ice_aqc_opc_write_i2c = 0x06E3,
ice_aqc_opc_set_port_id_led = 0x06E9,
ice_aqc_opc_set_gpio = 0x06EC,
ice_aqc_opc_get_gpio = 0x06ED,
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 9619bdb9e49a..1999c19a786e 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4823,7 +4823,7 @@ ice_aq_read_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
int status;
ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_read_i2c);
- cmd = &desc.params.read_i2c;
+ cmd = &desc.params.read_write_i2c;
if (!data)
return -EINVAL;
@@ -4850,6 +4850,55 @@ ice_aq_read_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
return status;
}
+/**
+ * ice_aq_write_i2c
+ * @hw: pointer to the hw struct
+ * @topo_addr: topology address for a device to communicate with
+ * @bus_addr: 7-bit I2C bus address
+ * @addr: I2C memory address (I2C offset) with up to 16 bits
+ * @params: I2C parameters: bit [4] - I2C address type, bits [3:0] - data size to write (0-7 bytes)
+ * @data: pointer to data (0 to 4 bytes) to be written to the I2C device
+ * @cd: pointer to command details structure or NULL
+ *
+ * Write I2C (0x06E3)
+ *
+ * * Return:
+ * * 0 - Successful write to the i2c device
+ * * -EINVAL - Data size greater than 4 bytes
+ * * -EIO - FW error
+ */
+int
+ice_aq_write_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
+ u16 bus_addr, __le16 addr, u8 params, u8 *data,
+ struct ice_sq_cd *cd)
+{
+ struct ice_aq_desc desc = { 0 };
+ struct ice_aqc_i2c *cmd;
+ unsigned int i;
+ u8 data_size;
+
+ ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_write_i2c);
+ cmd = &desc.params.read_write_i2c;
+
+ data_size = FIELD_GET(ICE_AQC_I2C_DATA_SIZE_M, params);
+
+ /* data_size limited to 4 */
+ if (data_size > 4)
+ return -EINVAL;
+
+ cmd->i2c_bus_addr = cpu_to_le16(bus_addr);
+ cmd->topo_addr = topo_addr;
+ cmd->i2c_params = params;
+ cmd->i2c_addr = addr;
+
+ for (i = 0; i < data_size; i++) {
+ cmd->i2c_data[i] = *data;
+ data++;
+ }
+
+ return ice_aq_send_cmd(hw, &desc, NULL, 0, cd);
+}
+
/**
* ice_aq_set_driver_param - Set driver parameter to share via firmware
* @hw: pointer to the HW struct
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 872ea7d2332d..61b7c60db689 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -214,5 +214,9 @@ int
ice_aq_read_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
u16 bus_addr, __le16 addr, u8 params, u8 *data,
struct ice_sq_cd *cd);
+int
+ice_aq_write_i2c(struct ice_hw *hw, struct ice_aqc_link_topo_addr topo_addr,
+ u16 bus_addr, __le16 addr, u8 params, u8 *data,
+ struct ice_sq_cd *cd);
bool ice_fw_supports_report_dflt_cfg(struct ice_hw *hw);
#endif /* _ICE_COMMON_H_ */
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 1/3] ice: remove u16 arithmetic in ice_gnss
From: Tony Nguyen @ 2022-05-17 21:19 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet
Cc: Karol Kolacinski, netdev, anthony.l.nguyen, richardcochran,
Gurucharan
In-Reply-To: <20220517211935.1949447-1-anthony.l.nguyen@intel.com>
From: Karol Kolacinski <karol.kolacinski@intel.com>
Change u16 to unsigned int where arithmetic occurs.
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
Tested-by: Gurucharan <gurucharanx.g@intel.com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_gnss.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_gnss.c b/drivers/net/ethernet/intel/ice/ice_gnss.c
index 57586a2e6dec..c6d755f707aa 100644
--- a/drivers/net/ethernet/intel/ice/ice_gnss.c
+++ b/drivers/net/ethernet/intel/ice/ice_gnss.c
@@ -17,13 +17,13 @@ static void ice_gnss_read(struct kthread_work *work)
struct gnss_serial *gnss = container_of(work, struct gnss_serial,
read_work.work);
struct ice_aqc_link_topo_addr link_topo;
- u8 i2c_params, bytes_read;
+ unsigned int i, bytes_read, data_len;
struct tty_port *port;
struct ice_pf *pf;
struct ice_hw *hw;
__be16 data_len_b;
char *buf = NULL;
- u16 i, data_len;
+ u8 i2c_params;
int err = 0;
pf = gnss->back;
@@ -65,7 +65,7 @@ static void ice_gnss_read(struct kthread_work *work)
mdelay(10);
}
- data_len = min(data_len, (u16)PAGE_SIZE);
+ data_len = min_t(typeof(data_len), data_len, PAGE_SIZE);
data_len = tty_buffer_request_room(port, data_len);
if (!data_len) {
err = -ENOMEM;
@@ -74,9 +74,10 @@ static void ice_gnss_read(struct kthread_work *work)
/* Read received data */
for (i = 0; i < data_len; i += bytes_read) {
- u16 bytes_left = data_len - i;
+ unsigned int bytes_left = data_len - i;
- bytes_read = min_t(typeof(bytes_left), bytes_left, ICE_MAX_I2C_DATA_SIZE);
+ bytes_read = min_t(typeof(bytes_left), bytes_left,
+ ICE_MAX_I2C_DATA_SIZE);
err = ice_aq_read_i2c(hw, link_topo, ICE_GNSS_UBX_I2C_BUS_ADDR,
cpu_to_le16(ICE_GNSS_UBX_EMPTY_DATA),
--
2.35.1
^ permalink raw reply related
* [PATCH net-next 0/3][pull request] 100GbE Intel Wired LAN Driver Updates 2022-05-17
From: Tony Nguyen @ 2022-05-17 21:19 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet; +Cc: Tony Nguyen, netdev, richardcochran
This series contains updates to ice driver only.
Karol changes calculation of u16 to unsigned int for GNSS related
calculations. He also adds implementation for GNSS write; data is
written to the GNSS module through TTY device using u-blox UBX protocol.
The following are changes since commit 65a9dedc11d615d8f104a48d38b4fa226967b4ed:
net: phy: marvell: Add errata section 5.1 for Alaska PHY
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 100GbE
Karol Kolacinski (3):
ice: remove u16 arithmetic in ice_gnss
ice: add i2c write command
ice: add write functionality for GNSS TTY
drivers/net/ethernet/intel/ice/ice.h | 4 +-
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 7 +-
drivers/net/ethernet/intel/ice/ice_common.c | 51 +++-
drivers/net/ethernet/intel/ice/ice_common.h | 4 +
drivers/net/ethernet/intel/ice/ice_gnss.c | 252 +++++++++++++++---
drivers/net/ethernet/intel/ice/ice_gnss.h | 26 +-
6 files changed, 304 insertions(+), 40 deletions(-)
--
2.35.1
^ permalink raw reply
* [question] bonding: should assert dormant for active protocols like LACP?
From: Jonathan Toppins @ 2022-05-17 21:17 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
So running the following script:
--%<-----
ip link add name link-bond0 type veth peer name link-end0
ip link add bond0 type bond mode 4 miimon 100
ip link set link-bond0 master bond0 down
ip netns add n1
ip link set link-end0 netns n1 up
ip link set bond0 up
cat /sys/class/net/bond0/bonding/ad_partner_mac
cat /sys/class/net/bond0/operstate
--%<-----
The bond reports its operstate to be "up" even though the bond will
never be able to establish an LACP partner. Should bonding for active
protocols, LACP, assert dormant[0] until the protocol has established
and frames actually are passed?
Having a predictable operstate where up actually means frames will
attempt to be delivered would make management applications, f.e. Network
Manager, easier to write. I have developers asking me what detailed
states for LACP should they be looking for to determine when an LACP
bond is "up". This seems like an incorrect implementation of operstate
and RFC2863 3.1.12.
Does anyone see why this would be a bad idea?
-Jon
[0] Documentation/networking/operstates.rst
^ permalink raw reply
* Re: [RFC net-next] bonding: netlink error message support for options
From: Jay Vosburgh @ 2022-05-17 21:11 UTC (permalink / raw)
To: Jonathan Toppins
Cc: netdev, Veaceslav Falico, Andy Gospodarek, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
In-Reply-To: <5a6ba6f14b0fad6d4ba077a5230ee71cbf970934.1652819479.git.jtoppins@redhat.com>
Jonathan Toppins <jtoppins@redhat.com> wrote:
>Add support for reporting errors via extack in both bond_newlink
>and bond_changelink.
>
>Instead of having to look in the kernel log for why an option was not
>correct just report the error to the user via the extack variable.
>
>What is currently reported today:
> ip link add bond0 type bond
> ip link set bond0 up
> ip link set bond0 type bond mode 4
> RTNETLINK answers: Device or resource busy
>
>After this change:
> ip link add bond0 type bond
> ip link set bond0 up
> ip link set bond0 type bond mode 4
> Error: option mode: unable to set because the bond device is up.
>
>Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
>---
>
>Notes:
> This is an RFC because the current NL_SET_ERR_MSG() macros do not support
> printf like semantics so I rolled my own buffer setting in __bond_opt_set().
> The issue is I could not quite figure out the life-cycle of the buffer, if
> rtnl lock is held until after the text buffer is copied into the packet
> then we are ok, otherwise, some other type of buffer management scheme will
> be needed as this could result in corrupted error messages when modifying
> multiple bonds.
If I'm reading the code correctly, rtnl isn't held that long.
Once the ->doit() returns, rtnl is dropped, but the copy happens later:
rtnetlink_rcv()
netlink_rcv_skb(skb, &rtnetlink_rcv_msg)
rtnetlink_rcv_msg() [ as cb(skb, nlh, &extack) ]
rtnl_lock()
link->doit() [ rtnl_setlink, rtnl_newlink, et al ]
rtnl_unlock()
netlink_ack()
inside netlink_ack():
if (nlk_has_extack && extack) {
if (extack->_msg) {
WARN_ON(nla_put_string(skb, NLMSGERR_ATTR_MSG,
extack->_msg));
}
Even if the strings have to be constant (via NL_SET_ERR_MSG),
adding extack messages is likely still an improvement.
-J
> drivers/net/bonding/bond_netlink.c | 87 ++++++++++++++++++------------
> drivers/net/bonding/bond_options.c | 62 +++++++++++++--------
> include/net/bond_options.h | 2 +-
> 3 files changed, 96 insertions(+), 55 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index f427fa1737c7..418a4f3d00a3 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -151,7 +151,7 @@ static int bond_slave_changelink(struct net_device *bond_dev,
> snprintf(queue_id_str, sizeof(queue_id_str), "%s:%u\n",
> slave_dev->name, queue_id);
> bond_opt_initstr(&newval, queue_id_str);
>- err = __bond_opt_set(bond, BOND_OPT_QUEUE_ID, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_QUEUE_ID, &newval, extack);
> if (err)
> return err;
> }
>@@ -175,7 +175,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int mode = nla_get_u8(data[IFLA_BOND_MODE]);
>
> bond_opt_initval(&newval, mode);
>- err = __bond_opt_set(bond, BOND_OPT_MODE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_MODE, &newval, extack);
> if (err)
> return err;
> }
>@@ -192,7 +192,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> active_slave = slave_dev->name;
> }
> bond_opt_initstr(&newval, active_slave);
>- err = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -200,7 +201,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> miimon = nla_get_u32(data[IFLA_BOND_MIIMON]);
>
> bond_opt_initval(&newval, miimon);
>- err = __bond_opt_set(bond, BOND_OPT_MIIMON, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_MIIMON, &newval, extack);
> if (err)
> return err;
> }
>@@ -208,7 +209,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int updelay = nla_get_u32(data[IFLA_BOND_UPDELAY]);
>
> bond_opt_initval(&newval, updelay);
>- err = __bond_opt_set(bond, BOND_OPT_UPDELAY, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_UPDELAY, &newval, extack);
> if (err)
> return err;
> }
>@@ -216,7 +217,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int downdelay = nla_get_u32(data[IFLA_BOND_DOWNDELAY]);
>
> bond_opt_initval(&newval, downdelay);
>- err = __bond_opt_set(bond, BOND_OPT_DOWNDELAY, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_DOWNDELAY, &newval, extack);
> if (err)
> return err;
> }
>@@ -224,7 +225,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int delay = nla_get_u32(data[IFLA_BOND_PEER_NOTIF_DELAY]);
>
> bond_opt_initval(&newval, delay);
>- err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -232,7 +234,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int use_carrier = nla_get_u8(data[IFLA_BOND_USE_CARRIER]);
>
> bond_opt_initval(&newval, use_carrier);
>- err = __bond_opt_set(bond, BOND_OPT_USE_CARRIER, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_USE_CARRIER, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -240,12 +243,14 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int arp_interval = nla_get_u32(data[IFLA_BOND_ARP_INTERVAL]);
>
> if (arp_interval && miimon) {
>- netdev_err(bond->dev, "ARP monitoring cannot be used with MII monitoring\n");
>+ NL_SET_ERR_MSG(extack,
>+ "ARP monitoring cannot be used with MII monitoring");
> return -EINVAL;
> }
>
> bond_opt_initval(&newval, arp_interval);
>- err = __bond_opt_set(bond, BOND_OPT_ARP_INTERVAL, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_ARP_INTERVAL, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -264,7 +269,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
>
> bond_opt_initval(&newval, (__force u64)target);
> err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
>- &newval);
>+ &newval, extack);
> if (err)
> break;
> i++;
>@@ -297,7 +302,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
>
> bond_opt_initextra(&newval, &addr6, sizeof(addr6));
> err = __bond_opt_set(bond, BOND_OPT_NS_TARGETS,
>- &newval);
>+ &newval, extack);
> if (err)
> break;
> i++;
>@@ -312,12 +317,14 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int arp_validate = nla_get_u32(data[IFLA_BOND_ARP_VALIDATE]);
>
> if (arp_validate && miimon) {
>- netdev_err(bond->dev, "ARP validating cannot be used with MII monitoring\n");
>+ NL_SET_ERR_MSG(extack,
>+ "ARP validating cannot be used with MII monitoring");
> return -EINVAL;
> }
>
> bond_opt_initval(&newval, arp_validate);
>- err = __bond_opt_set(bond, BOND_OPT_ARP_VALIDATE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_ARP_VALIDATE, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -326,7 +333,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u32(data[IFLA_BOND_ARP_ALL_TARGETS]);
>
> bond_opt_initval(&newval, arp_all_targets);
>- err = __bond_opt_set(bond, BOND_OPT_ARP_ALL_TARGETS, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_ARP_ALL_TARGETS, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -340,7 +348,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> primary = dev->name;
>
> bond_opt_initstr(&newval, primary);
>- err = __bond_opt_set(bond, BOND_OPT_PRIMARY, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_PRIMARY, &newval, extack);
> if (err)
> return err;
> }
>@@ -349,7 +357,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_PRIMARY_RESELECT]);
>
> bond_opt_initval(&newval, primary_reselect);
>- err = __bond_opt_set(bond, BOND_OPT_PRIMARY_RESELECT, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_PRIMARY_RESELECT, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -358,7 +367,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_FAIL_OVER_MAC]);
>
> bond_opt_initval(&newval, fail_over_mac);
>- err = __bond_opt_set(bond, BOND_OPT_FAIL_OVER_MAC, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_FAIL_OVER_MAC, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -367,7 +377,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_XMIT_HASH_POLICY]);
>
> bond_opt_initval(&newval, xmit_hash_policy);
>- err = __bond_opt_set(bond, BOND_OPT_XMIT_HASH, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_XMIT_HASH, &newval, extack);
> if (err)
> return err;
> }
>@@ -376,7 +386,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u32(data[IFLA_BOND_RESEND_IGMP]);
>
> bond_opt_initval(&newval, resend_igmp);
>- err = __bond_opt_set(bond, BOND_OPT_RESEND_IGMP, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_RESEND_IGMP, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -385,7 +396,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_NUM_PEER_NOTIF]);
>
> bond_opt_initval(&newval, num_peer_notif);
>- err = __bond_opt_set(bond, BOND_OPT_NUM_PEER_NOTIF, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_NUM_PEER_NOTIF, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -394,7 +406,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_ALL_SLAVES_ACTIVE]);
>
> bond_opt_initval(&newval, all_slaves_active);
>- err = __bond_opt_set(bond, BOND_OPT_ALL_SLAVES_ACTIVE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_ALL_SLAVES_ACTIVE, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -403,7 +416,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u32(data[IFLA_BOND_MIN_LINKS]);
>
> bond_opt_initval(&newval, min_links);
>- err = __bond_opt_set(bond, BOND_OPT_MINLINKS, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_MINLINKS, &newval, extack);
> if (err)
> return err;
> }
>@@ -412,7 +425,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u32(data[IFLA_BOND_LP_INTERVAL]);
>
> bond_opt_initval(&newval, lp_interval);
>- err = __bond_opt_set(bond, BOND_OPT_LP_INTERVAL, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_LP_INTERVAL, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -421,7 +435,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u32(data[IFLA_BOND_PACKETS_PER_SLAVE]);
>
> bond_opt_initval(&newval, packets_per_slave);
>- err = __bond_opt_set(bond, BOND_OPT_PACKETS_PER_SLAVE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_PACKETS_PER_SLAVE, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -430,7 +445,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int lacp_active = nla_get_u8(data[IFLA_BOND_AD_LACP_ACTIVE]);
>
> bond_opt_initval(&newval, lacp_active);
>- err = __bond_opt_set(bond, BOND_OPT_LACP_ACTIVE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_LACP_ACTIVE, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -440,7 +456,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_AD_LACP_RATE]);
>
> bond_opt_initval(&newval, lacp_rate);
>- err = __bond_opt_set(bond, BOND_OPT_LACP_RATE, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_LACP_RATE, &newval, extack);
> if (err)
> return err;
> }
>@@ -449,7 +465,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u8(data[IFLA_BOND_AD_SELECT]);
>
> bond_opt_initval(&newval, ad_select);
>- err = __bond_opt_set(bond, BOND_OPT_AD_SELECT, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_AD_SELECT, &newval, extack);
> if (err)
> return err;
> }
>@@ -458,7 +474,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u16(data[IFLA_BOND_AD_ACTOR_SYS_PRIO]);
>
> bond_opt_initval(&newval, actor_sys_prio);
>- err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYS_PRIO, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYS_PRIO, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -467,7 +484,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> nla_get_u16(data[IFLA_BOND_AD_USER_PORT_KEY]);
>
> bond_opt_initval(&newval, port_key);
>- err = __bond_opt_set(bond, BOND_OPT_AD_USER_PORT_KEY, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_AD_USER_PORT_KEY, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -477,7 +495,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
>
> bond_opt_initval(&newval,
> nla_get_u64(data[IFLA_BOND_AD_ACTOR_SYSTEM]));
>- err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYSTEM, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYSTEM, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -485,7 +504,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int dynamic_lb = nla_get_u8(data[IFLA_BOND_TLB_DYNAMIC_LB]);
>
> bond_opt_initval(&newval, dynamic_lb);
>- err = __bond_opt_set(bond, BOND_OPT_TLB_DYNAMIC_LB, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_TLB_DYNAMIC_LB, &newval,
>+ extack);
> if (err)
> return err;
> }
>@@ -494,7 +514,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> int missed_max = nla_get_u8(data[IFLA_BOND_MISSED_MAX]);
>
> bond_opt_initval(&newval, missed_max);
>- err = __bond_opt_set(bond, BOND_OPT_MISSED_MAX, &newval);
>+ err = __bond_opt_set(bond, BOND_OPT_MISSED_MAX, &newval,
>+ extack);
> if (err)
> return err;
> }
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 64f7db2627ce..4a95503384a3 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -636,7 +636,8 @@ static int bond_opt_check_deps(struct bonding *bond,
> }
>
> static void bond_opt_dep_print(struct bonding *bond,
>- const struct bond_option *opt)
>+ const struct bond_option *opt,
>+ char *buf, size_t bufsize)
> {
> const struct bond_opt_value *modeval;
> struct bond_params *params;
>@@ -644,16 +645,18 @@ static void bond_opt_dep_print(struct bonding *bond,
> params = &bond->params;
> modeval = bond_opt_get_val(BOND_OPT_MODE, params->mode);
> if (test_bit(params->mode, &opt->unsuppmodes))
>- netdev_err(bond->dev, "option %s: mode dependency failed, not supported in mode %s(%llu)\n",
>- opt->name, modeval->string, modeval->value);
>+ scnprintf(buf, bufsize, "option %s: mode dependency failed, not supported in mode %s(%llu)\n",
>+ opt->name, modeval->string, modeval->value);
> }
>
> static void bond_opt_error_interpret(struct bonding *bond,
> const struct bond_option *opt,
>- int error, const struct bond_opt_value *val)
>+ int error, const struct bond_opt_value *val,
>+ char *buf, size_t bufsize)
> {
> const struct bond_opt_value *minval, *maxval;
> char *p;
>+ int i = 0;
>
> switch (error) {
> case -EINVAL:
>@@ -663,38 +666,45 @@ static void bond_opt_error_interpret(struct bonding *bond,
> p = strchr(val->string, '\n');
> if (p)
> *p = '\0';
>- netdev_err(bond->dev, "option %s: invalid value (%s)\n",
>- opt->name, val->string);
>+ i = scnprintf(buf, bufsize,
>+ "option %s: invalid value (%s)",
>+ opt->name, val->string);
> } else {
>- netdev_err(bond->dev, "option %s: invalid value (%llu)\n",
>- opt->name, val->value);
>+ i = scnprintf(buf, bufsize,
>+ "option %s: invalid value (%llu)",
>+ opt->name, val->value);
> }
> }
> minval = bond_opt_get_flags(opt, BOND_VALFLAG_MIN);
> maxval = bond_opt_get_flags(opt, BOND_VALFLAG_MAX);
> if (!maxval)
> break;
>- netdev_err(bond->dev, "option %s: allowed values %llu - %llu\n",
>- opt->name, minval ? minval->value : 0, maxval->value);
>+ if (i) {
>+ // index buf to overwirte '\n' from above
>+ buf = &buf[i];
>+ bufsize -= i;
>+ }
>+ scnprintf(buf, bufsize, " allowed values %llu - %llu",
>+ minval ? minval->value : 0, maxval->value);
> break;
> case -EACCES:
>- bond_opt_dep_print(bond, opt);
>+ bond_opt_dep_print(bond, opt, buf, bufsize);
> break;
> case -ENOTEMPTY:
>- netdev_err(bond->dev, "option %s: unable to set because the bond device has slaves\n",
>- opt->name);
>+ scnprintf(buf, bufsize, "option %s: unable to set because the bond device has slaves",
>+ opt->name);
> break;
> case -EBUSY:
>- netdev_err(bond->dev, "option %s: unable to set because the bond device is up\n",
>- opt->name);
>+ scnprintf(buf, bufsize, "option %s: unable to set because the bond device is up",
>+ opt->name);
> break;
> case -ENODEV:
> if (val && val->string) {
> p = strchr(val->string, '\n');
> if (p)
> *p = '\0';
>- netdev_err(bond->dev, "option %s: interface %s does not exist!\n",
>- opt->name, val->string);
>+ scnprintf(buf, bufsize, "option %s: interface %s does not exist!",
>+ opt->name, val->string);
> }
> break;
> default:
>@@ -713,7 +723,8 @@ static void bond_opt_error_interpret(struct bonding *bond,
> * must be obtained before calling this function.
> */
> int __bond_opt_set(struct bonding *bond,
>- unsigned int option, struct bond_opt_value *val)
>+ unsigned int option, struct bond_opt_value *val,
>+ struct netlink_ext_ack *extack)
> {
> const struct bond_opt_value *retval = NULL;
> const struct bond_option *opt;
>@@ -734,8 +745,17 @@ int __bond_opt_set(struct bonding *bond,
> }
> ret = opt->set(bond, retval);
> out:
>- if (ret)
>- bond_opt_error_interpret(bond, opt, ret, val);
>+ if (ret) {
>+ static char buf[120];
>+ buf[0] = '\0';
>+ bond_opt_error_interpret(bond, opt, ret, val, buf, sizeof(buf));
>+ if (buf[0] != '\0') {
>+ if (extack)
>+ extack->_msg = buf;
>+ else
>+ netdev_err(bond->dev, "Error: %s\n", buf);
>+ }
>+ }
>
> return ret;
> }
>@@ -757,7 +777,7 @@ int __bond_opt_set_notify(struct bonding *bond,
>
> ASSERT_RTNL();
>
>- ret = __bond_opt_set(bond, option, val);
>+ ret = __bond_opt_set(bond, option, val, NULL);
>
> if (!ret && (bond->dev->reg_state == NETREG_REGISTERED))
> call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index 61b49063791c..ae38557adc25 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -107,7 +107,7 @@ struct bond_option {
> };
>
> int __bond_opt_set(struct bonding *bond, unsigned int option,
>- struct bond_opt_value *val);
>+ struct bond_opt_value *val, struct netlink_ext_ack *extack);
> int __bond_opt_set_notify(struct bonding *bond, unsigned int option,
> struct bond_opt_value *val);
> int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf);
>--
>2.27.0
>
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net: ipqess: introduce the Qualcomm IPQESS driver
From: Christophe JAILLET @ 2022-05-17 21:03 UTC (permalink / raw)
To: Maxime Chevallier, davem, Rob Herring
Cc: netdev, linux-kernel, devicetree, thomas.petazzoni, Andrew Lunn,
Florian Fainelli, Heiner Kallweit, Russell King, linux-arm-kernel,
Vladimir Oltean, Luka Perkov, Robert Marko
In-Reply-To: <20220514150656.122108-2-maxime.chevallier@bootlin.com>
Le 14/05/2022 à 17:06, Maxime Chevallier a écrit :
> The Qualcomm IPQESS controller is a simple 1G Ethernet controller found
> on the IPQ4019 chip. This controller has some specificities, in that the
> IPQ4019 platform that includes that controller also has an internal
> switch, based on the QCA8K IP.
>
> It is connected to that switch through an internal link, and doesn't
> expose directly any external interface, hence it only supports the
> PHY_INTERFACE_MODE_INTERNAL for now.
>
> It has 16 RX and TX queues, with a very basic RSS fanout configured at
> init time.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> V1->V2 :
> - Reworked the init sequence, following Andrew's comments
> - Added clock and reset support
> - Reworked the error paths
> - Added extra endianness wrappers to fix sparse warnings
>
> MAINTAINERS | 6 +
> drivers/net/ethernet/qualcomm/Kconfig | 11 +
> drivers/net/ethernet/qualcomm/Makefile | 2 +
> drivers/net/ethernet/qualcomm/ipqess/Makefile | 8 +
> drivers/net/ethernet/qualcomm/ipqess/ipqess.c | 1269 +++++++++++++++++
> drivers/net/ethernet/qualcomm/ipqess/ipqess.h | 518 +++++++
> .../ethernet/qualcomm/ipqess/ipqess_ethtool.c | 168 +++
> 7 files changed, 1982 insertions(+)
> create mode 100644 drivers/net/ethernet/qualcomm/ipqess/Makefile
> create mode 100644 drivers/net/ethernet/qualcomm/ipqess/ipqess.c
> create mode 100644 drivers/net/ethernet/qualcomm/ipqess/ipqess.h
> create mode 100644 drivers/net/ethernet/qualcomm/ipqess/ipqess_ethtool.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9b0480f1b153..29e6ec4f975a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16308,6 +16308,12 @@ L: netdev@vger.kernel.org
> S: Maintained
> F: drivers/net/ethernet/qualcomm/emac/
>
> +QUALCOMM IPQESS ETHERNET DRIVER
> +M: Maxime Chevallier <maxime.chevallier@bootlin.com>
> +L: netdev@vger.kernel.org
> +S: Maintained
> +F: drivers/net/ethernet/qualcomm/ipqess/
> +
> QUALCOMM ETHQOS ETHERNET DRIVER
> M: Vinod Koul <vkoul@kernel.org>
> L: netdev@vger.kernel.org
> diff --git a/drivers/net/ethernet/qualcomm/Kconfig b/drivers/net/ethernet/qualcomm/Kconfig
> index a4434eb38950..a723ddbea248 100644
> --- a/drivers/net/ethernet/qualcomm/Kconfig
> +++ b/drivers/net/ethernet/qualcomm/Kconfig
> @@ -60,6 +60,17 @@ config QCOM_EMAC
> low power, Receive-Side Scaling (RSS), and IEEE 1588-2008
> Precision Clock Synchronization Protocol.
>
> +config QCOM_IPQ4019_ESS_EDMA
> + tristate "Qualcomm Atheros IPQ4019 ESS EDMA support"
> + depends on OF
> + select PHYLINK
> + help
> + This driver supports the Qualcomm Atheros IPQ40xx built-in
> + ESS EDMA ethernet controller.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called ipqess.
> +
> source "drivers/net/ethernet/qualcomm/rmnet/Kconfig"
>
> endif # NET_VENDOR_QUALCOMM
> diff --git a/drivers/net/ethernet/qualcomm/Makefile b/drivers/net/ethernet/qualcomm/Makefile
> index 9250976dd884..db463c9ea1f9 100644
> --- a/drivers/net/ethernet/qualcomm/Makefile
> +++ b/drivers/net/ethernet/qualcomm/Makefile
> @@ -11,4 +11,6 @@ qcauart-objs := qca_uart.o
>
> obj-y += emac/
>
> +obj-$(CONFIG_QCOM_IPQ4019_ESS_EDMA) += ipqess/
> +
> obj-$(CONFIG_RMNET) += rmnet/
> diff --git a/drivers/net/ethernet/qualcomm/ipqess/Makefile b/drivers/net/ethernet/qualcomm/ipqess/Makefile
> new file mode 100644
> index 000000000000..4f2db7283ebf
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/ipqess/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for the IPQ ESS driver
> +#
> +
> +obj-$(CONFIG_QCOM_IPQ4019_ESS_EDMA) += ipq_ess.o
> +
> +ipq_ess-objs := ipqess.o ipqess_ethtool.o
> diff --git a/drivers/net/ethernet/qualcomm/ipqess/ipqess.c b/drivers/net/ethernet/qualcomm/ipqess/ipqess.c
> new file mode 100644
> index 000000000000..b11f11f23c11
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/ipqess/ipqess.c
> @@ -0,0 +1,1269 @@
> +// SPDX-License-Identifier: GPL-2.0 OR ISC
> +/* Copyright (c) 2014 - 2017, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2017 - 2018, John Crispin <john@phrozen.org>
> + * Copyright (c) 2018 - 2019, Christian Lamparter <chunkeey@gmail.com>
> + * Copyright (c) 2020 - 2021, Gabor Juhos <j4g8y7@gmail.com>
> + * Copyright (c) 2021 - 2022, Maxime Chevallier <maxime.chevallier@bootlin.com>
> + *
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/if_vlan.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_mdio.h>
> +#include <linux/of_net.h>
> +#include <linux/phylink.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/skbuff.h>
> +#include <linux/vmalloc.h>
> +#include <net/checksum.h>
> +#include <net/ip6_checksum.h>
> +
> +#include "ipqess.h"
> +
> +#define IPQESS_RRD_SIZE 16
> +#define IPQESS_NEXT_IDX(X, Y) (((X) + 1) & ((Y) - 1))
> +#define IPQESS_TX_DMA_BUF_LEN 0x3fff
> +
> +static void ipqess_w32(struct ipqess *ess, u32 reg, u32 val)
> +{
> + writel(val, ess->hw_addr + reg);
> +}
> +
> +static u32 ipqess_r32(struct ipqess *ess, u16 reg)
> +{
> + return readl(ess->hw_addr + reg);
> +}
> +
> +static void ipqess_m32(struct ipqess *ess, u32 mask, u32 val, u16 reg)
> +{
> + u32 _val = ipqess_r32(ess, reg);
> +
> + _val &= ~mask;
> + _val |= val;
> +
> + ipqess_w32(ess, reg, _val);
> +}
> +
> +void ipqess_update_hw_stats(struct ipqess *ess)
> +{
> + u32 *p;
> + u32 stat;
> + int i;
> +
> + lockdep_assert_held(&ess->stats_lock);
> +
> + p = (u32 *)&ess->ipqess_stats;
> + for (i = 0; i < IPQESS_MAX_TX_QUEUE; i++) {
> + stat = ipqess_r32(ess, IPQESS_REG_TX_STAT_PKT_Q(i));
> + *p += stat;
> + p++;
> + }
> +
> + for (i = 0; i < IPQESS_MAX_TX_QUEUE; i++) {
> + stat = ipqess_r32(ess, IPQESS_REG_TX_STAT_BYTE_Q(i));
> + *p += stat;
> + p++;
> + }
> +
> + for (i = 0; i < IPQESS_MAX_RX_QUEUE; i++) {
> + stat = ipqess_r32(ess, IPQESS_REG_RX_STAT_PKT_Q(i));
> + *p += stat;
> + p++;
> + }
> +
> + for (i = 0; i < IPQESS_MAX_RX_QUEUE; i++) {
> + stat = ipqess_r32(ess, IPQESS_REG_RX_STAT_BYTE_Q(i));
> + *p += stat;
> + p++;
> + }
> +}
> +
> +static int ipqess_tx_ring_alloc(struct ipqess *ess)
> +{
> + struct device *dev = &ess->pdev->dev;
> + int i;
> +
> + for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) {
> + struct ipqess_tx_ring *tx_ring = &ess->tx_ring[i];
> + size_t size;
> + u32 idx;
> +
> + tx_ring->ess = ess;
> + tx_ring->ring_id = i;
> + tx_ring->idx = i * 4;
> + tx_ring->count = IPQESS_TX_RING_SIZE;
> + tx_ring->nq = netdev_get_tx_queue(ess->netdev, i);
> +
> + size = sizeof(struct ipqess_buf) * IPQESS_TX_RING_SIZE;
> + tx_ring->buf = devm_kzalloc(dev, size, GFP_KERNEL);
> + if (!tx_ring->buf) {
> + netdev_err(ess->netdev, "buffer alloc of tx ring failed");
> + return -ENOMEM;
> + }
> +
> + size = sizeof(struct ipqess_tx_desc) * IPQESS_TX_RING_SIZE;
> + tx_ring->hw_desc = dmam_alloc_coherent(dev, size, &tx_ring->dma,
> + GFP_KERNEL | __GFP_ZERO);
Hi,
Nitpicking: I think that __GFP_ZERO is useless (and harmless) because
dma_alloc_coherent() always zeroes the memory that is allocated.
CJ
^ permalink raw reply
* [PATCH v2 2/2] octeon_ep: Fix irq releasing in the error handling path of octep_request_irqs()
From: Christophe JAILLET @ 2022-05-17 20:59 UTC (permalink / raw)
To: vburru, aayarekar, davem, edumazet, kuba, pabeni, sburla
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
In-Reply-To: <cover.1652819974.git.christophe.jaillet@wanadoo.fr>
When taken, the error handling path does not undo correctly what has
already been allocated.
Introduce a new loop index, 'j', in order to simplify the error handling
path and rewrite part of it.
It is now written with the same logic and intermediate variables used
when resources are allocated. This is much more straightforward.
Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v2: Introduce 'j' and use it in the 2nd for loop
Rewrite the error handling path (Dan Carpenter)
v1:
https://lore.kernel.org/all/a1b6f082fff4e68007914577961113bc452c8030.1652629833.git.christophe.jaillet@wanadoo.fr/
---
.../ethernet/marvell/octeon_ep/octep_main.c | 25 +++++++++++--------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 6b60a03574a0..a9b82d221780 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -202,7 +202,7 @@ static int octep_request_irqs(struct octep_device *oct)
struct msix_entry *msix_entry;
char **non_ioq_msix_names;
int num_non_ioq_msix;
- int ret, i;
+ int ret, i, j;
num_non_ioq_msix = CFG_GET_NON_IOQ_MSIX(oct->conf);
non_ioq_msix_names = CFG_GET_NON_IOQ_MSIX_NAMES(oct->conf);
@@ -233,23 +233,23 @@ static int octep_request_irqs(struct octep_device *oct)
}
/* Request IRQs for Tx/Rx queues */
- for (i = 0; i < oct->num_oqs; i++) {
- ioq_vector = oct->ioq_vector[i];
- msix_entry = &oct->msix_entries[i + num_non_ioq_msix];
+ for (j = 0; j < oct->num_oqs; j++) {
+ ioq_vector = oct->ioq_vector[j];
+ msix_entry = &oct->msix_entries[j + num_non_ioq_msix];
snprintf(ioq_vector->name, sizeof(ioq_vector->name),
- "%s-q%d", netdev->name, i);
+ "%s-q%d", netdev->name, j);
ret = request_irq(msix_entry->vector,
octep_ioq_intr_handler, 0,
ioq_vector->name, ioq_vector);
if (ret) {
netdev_err(netdev,
"request_irq failed for Q-%d; err=%d",
- i, ret);
+ j, ret);
goto ioq_irq_err;
}
- cpumask_set_cpu(i % num_online_cpus(),
+ cpumask_set_cpu(j % num_online_cpus(),
&ioq_vector->affinity_mask);
irq_set_affinity_hint(msix_entry->vector,
&ioq_vector->affinity_mask);
@@ -257,10 +257,13 @@ static int octep_request_irqs(struct octep_device *oct)
return 0;
ioq_irq_err:
- while (i > num_non_ioq_msix) {
- --i;
- irq_set_affinity_hint(oct->msix_entries[i].vector, NULL);
- free_irq(oct->msix_entries[i].vector, oct->ioq_vector[i]);
+ while (j) {
+ --j;
+ ioq_vector = oct->ioq_vector[j];
+ msix_entry = &oct->msix_entries[j + num_non_ioq_msix];
+
+ irq_set_affinity_hint(msix_entry->vector, NULL);
+ free_irq(msix_entry->vector, ioq_vector);
}
non_ioq_irq_err:
while (i) {
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] octeon_ep: Fix a memory leak in the error handling path of octep_request_irqs()
From: Christophe JAILLET @ 2022-05-17 20:59 UTC (permalink / raw)
To: vburru, aayarekar, davem, edumazet, kuba, pabeni, sburla
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
In-Reply-To: <cover.1652819974.git.christophe.jaillet@wanadoo.fr>
'oct->non_ioq_irq_names' is not freed in the error handling path of
octep_request_irqs().
Add the missing kfree().
Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Acked-by: Veerasenareddy Burru <vburru@marvell.com>
---
v2: Add Acked-by tag
v1:
https://lore.kernel.org/all/78dcfbb5d22328bc83edbfc74af10c3625c54087.1652629833.git.christophe.jaillet@wanadoo.fr/
---
drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index e020c81f3455..6b60a03574a0 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -267,6 +267,8 @@ static int octep_request_irqs(struct octep_device *oct)
--i;
free_irq(oct->msix_entries[i].vector, oct);
}
+ kfree(oct->non_ioq_irq_names);
+ oct->non_ioq_irq_names = NULL;
alloc_err:
return -1;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/2] octeon_ep: Fix the error handling path of octep_request_irqs()
From: Christophe JAILLET @ 2022-05-17 20:59 UTC (permalink / raw)
To: vburru, aayarekar, davem, edumazet, kuba, pabeni, sburla
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
I send a small serie to ease review and because I'm sighly less confident with
the 2nd patch.
They are related to the same Fixes: tag, so they obviously could be merged if
it is preferred.
Details on modification of this v2 are given in each patch.
Christophe JAILLET (2):
octeon_ep: Fix a memory leak in the error handling path of
octep_request_irqs()
octeon_ep: Fix irq releasing in the error handling path of
octep_request_irqs()
.../ethernet/marvell/octeon_ep/octep_main.c | 27 +++++++++++--------
1 file changed, 16 insertions(+), 11 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] net: dsa: add out-of-band tagging protocol
From: Jakub Kicinski @ 2022-05-17 20:58 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, Rob Herring, netdev, linux-kernel, devicetree,
thomas.petazzoni, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
Russell King, linux-arm-kernel, Vladimir Oltean, Luka Perkov,
Robert Marko
In-Reply-To: <20220517085355.4fab54b3@pc-20.home>
On Tue, 17 May 2022 08:53:55 +0200 Maxime Chevallier wrote:
> > This must had been asked on v1 but there's no trace of it in the
> > current submission afaict...
>
> No you're correct, this wasn't explained.
>
> > If the tag is passed in the descriptor how is this not a pure
> > switchdev driver? The explanation must be preserved somehow.
>
> The main reason is that although the MAC and switch are rightly coupled
> on that platform, the switch is actually a QC8K that can live on it's
> own, as an external switch. Here, it's just a slightly modified version
> of this IP.
>
> The same goes for the MAC IP, but so far we don't support any other
> platform that have the MAC as a standalone controller. As far as we can
> tell, platforms that have this MAC also include a QCA8K, but the
> datasheet also mentions other modes (like outputing RGMII).
Got it, thanks! Please weave this justification more explicitly into
the cover letter.
> Is this valid to have it as a standalone ethernet driver in that
> situation ?
Quite possibly.. I won't pretend I've looked at the code, I defer
to the reviewers :)
^ permalink raw reply
* Re: Bonding problem on Intel X710 hardware
From: Jakub Kicinski @ 2022-05-17 20:45 UTC (permalink / raw)
To: Sven Anders; +Cc: netdev, intel-wired-lan, Tony Nguyen
In-Reply-To: <ad3e244d-2f87-c74b-1d40-c21e286a721c@anduras.de>
CC: intel
On Tue, 17 May 2022 16:23:16 +0200 Sven Anders wrote:
> Hello!
>
> This is a follow up to my question. I did not hear anything so far, but I tried
> to find some some information meanwhile.
>
> I've got a guess from somebody, that the error message "Error I40E_AQ_RC_EINVAL
> adding RX filters on PF, promiscuous mode forced on" maybe triggered, because
> I'm hitting a limit here.
>
> Somebody other said, that this seems to be an error in the "bonding driver", but
> I do not think so. Aside from that, there seem to be no special "bonding" mailing
> list anymore. So I will have to ask this questions here anyway...
>
> I want to understand the problem to classify it.
>
> 1) Why is this "error" issued? Do I really hit a limit and what is this current limit?
> 2) Is it really an error or is it more "a warning"?
> 3) Why is this error triggered only when changing the "ntuples filter" and not during
> the normal adding of VLANs?
> Remark: I can trigger the "ntuples fiilter" later on again and it still works.
>
> I also tried the latest 5.18-rc kernel with the same problem.
>
> Maybe somebody will find time and try to reproduce this?
> I will answer any questions...
>
> Regards
> Sven
>
> Am 12.05.22 um 16:05 schrieb Sven Anders:
> > Hello!
> >
> > I'm having problems setting up a bond in adaptive load balancing
> > mode (balance-alb, mode 6) on an intel X710 network adapter using
> > the i40e driver connected to an Aruba 2530-48G switch.
> > The network card has 4 on board ports.
> > I'm using 2 ports for the bond with 36 VLANs on it.
> >
> > The setup is correct, because it works without problems, if
> > I use the same setup with 1GBit Intel hardware (using the
> > e1000e driver, version 3.2.6-k, firmware 5.10-2).
> >
> > Data packets are only received sporadically. If I run the same test
> > with only one slave port, it works without problems.
> >
> > I debugged it down to the reception of the packets by the
> > network hardware.
> >
> > If I remove the number of VLANs under 8, almost all packets are
> > received. The fewer VLANs the better the receive rate.
> >
> > I suspected the hardware offloading operations to be the cause, so I
> > tried to disable them. It resulted in the following:
> >
> > If I turn of the "ntuple-filters" with
> > ethtool -K eth3 ntuple off
> > ethtool -K eth3 ntuple off
> > it will work.
> >
> > But if I do this I see the following errors in "dmesg":
> > i40e 0000:65:00.1: Error I40E_AQ_RC_EINVAL adding RX filters on PF, promiscuous mode forced on
> > i40e 0000:65:00.2: Error I40E_AQ_RC_EINVAL adding RX filters on PF, promiscuous mode forced on
> >
> > Disabling any any other offloading operations made no change.
> >
> > For me it seems, that the hardware filter is dropping packets because they
> > have the wrong values (mac-address ?).
> > Turning the "ntuple-filters" off, forces the network adapter to accept
> > all packets.
> >
> >
> > My questions:
> >
> > 1. Can anybody explain or confirm this?
> >
> > 2. Is the a correct method to force the adapter in promiscous mode?
> >
> > 3. Are the any special settings needed, if I use ALB bonding, which I missed?
> >
> >
> > Some details:
> > -------------
> >
> > Linux kernel 5.15.35-core2 on x86_64.
> >
> >
> > This is the hardware:
> > ---------------------
> > 4 port Ethernet controller:
> > Intel Corporation Ethernet Controller X710 for 10GBASE-T (rev 02)
> > 8086:15ff (rev 02)
> >
> > with
> >
> > driver: i40e
> > version: 5.15.35-core2
> > firmware-version: 8.60 0x8000bd80 1.3140.0
> > bus-info: 0000:65:00.2
> > supports-statistics: yes
> > supports-test: yes
> > supports-eeprom-access: yes
> > supports-register-dump: yes
> > supports-priv-flags: yes
> >
> >
> > This is current bonding configuration:
> > --------------------------------------
> > Ethernet Channel Bonding Driver: v5.15.35-core2
> >
> > Bonding Mode: adaptive load balancing
> > Primary Slave: None
> > Currently Active Slave: eth3
> > MII Status: up
> > MII Polling Interval (ms): 100
> > Up Delay (ms): 200
> > Down Delay (ms): 200
> > Peer Notification Delay (ms): 0
> >
> > Slave Interface: eth3
> > MII Status: up
> > Speed: 1000 Mbps
> > Duplex: full
> > Link Failure Count: 0
> > Permanent HW addr: 68:05:ca:f8:9c:42
> > Slave queue ID: 0
> >
> > Slave Interface: eth4
> > MII Status: up
> > Speed: 1000 Mbps
> > Duplex: full
> > Link Failure Count: 0
> > Permanent HW addr: 68:05:ca:f8:9c:41
> > Slave queue ID: 0
> >
> >
> > Regards
> > Sven Anders
> >
>
>
> Mit freundlichen Grüßen
> Sven Anders
>
^ permalink raw reply
* Re: [PATCH RESEND 2/5] dt-bindings: net: allow Ethernet devices as LED triggers
From: Rob Herring @ 2022-05-17 20:37 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Krzysztof Kozlowski, Pavel Machek, David S . Miller,
Jakub Kicinski, Paolo Abeni, Florian Fainelli, Hauke Mehrtens,
Jacek Anaszewski, devicetree, netdev, linux-leds,
linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list,
ansuelsmth, andrew, vivien.didelot, Vladimir Oltean,
Jonathan Corbet, John Crispin, linux-doc, Rafał Miłecki
In-Reply-To: <20220505135512.3486-3-zajec5@gmail.com>
On Thu, May 05, 2022 at 03:55:09PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This allows specifying Ethernet interfaces and switch ports as triggers
> for LEDs activity.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Documentation/devicetree/bindings/net/ethernet-controller.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index 4f15463611f8..ebeb4446d253 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -232,6 +232,9 @@ properties:
> required:
> - speed
>
> +allOf:
> + - $ref: /schemas/leds/trigger-source.yaml
There's no need to add this here. A device binding still has to list
'#trigger-source-cells' and set it's value to 0 or 1 cell.
Rob
^ permalink raw reply
* Re: [PATCH RESEND 1/5] dt-bindings: net: add bitfield defines for Ethernet speeds
From: Rob Herring @ 2022-05-17 20:35 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Krzysztof Kozlowski, Pavel Machek, David S . Miller,
Jakub Kicinski, Paolo Abeni, Florian Fainelli, Hauke Mehrtens,
Jacek Anaszewski, devicetree, netdev, linux-leds,
linux-arm-kernel, linux-kernel, bcm-kernel-feedback-list,
ansuelsmth, andrew, vivien.didelot, Vladimir Oltean,
Jonathan Corbet, John Crispin, linux-doc, Rafał Miłecki
In-Reply-To: <20220505135512.3486-2-zajec5@gmail.com>
On Thu, May 05, 2022 at 03:55:08PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This allows specifying multiple Ethernet speeds in a single DT uint32
> value.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> include/dt-bindings/net/eth.h | 27 +++++++++++++++++++++++++++
ethernet.h
> 1 file changed, 27 insertions(+)
> create mode 100644 include/dt-bindings/net/eth.h
>
> diff --git a/include/dt-bindings/net/eth.h b/include/dt-bindings/net/eth.h
> new file mode 100644
> index 000000000000..89caff09179b
> --- /dev/null
> +++ b/include/dt-bindings/net/eth.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
Dual license
> +/*
> + * Device Tree constants for the Ethernet
> + */
> +
> +#ifndef _DT_BINDINGS_ETH_H
> +#define _DT_BINDINGS_ETH_H
> +
> +#define SPEED_UNSPEC 0
> +#define SPEED_10 (1 << 0)
> +#define SPEED_100 (1 << 1)
> +#define SPEED_1000 (1 << 2)
> +#define SPEED_2000 (1 << 3)
> +#define SPEED_2500 (1 << 4)
> +#define SPEED_5000 (1 << 5)
> +#define SPEED_10000 (1 << 6)
> +#define SPEED_14000 (1 << 7)
> +#define SPEED_20000 (1 << 8)
> +#define SPEED_25000 (1 << 9)
> +#define SPEED_40000 (1 << 10)
> +#define SPEED_50000 (1 << 11)
> +#define SPEED_56000 (1 << 12)
> +#define SPEED_100000 (1 << 13)
> +#define SPEED_200000 (1 << 14)
> +#define SPEED_400000 (1 << 15)
These should probably have some namespace. ETH_*?
> +
> +#endif
> --
> 2.34.1
>
>
^ permalink raw reply
* [RFC net-next] bonding: netlink error message support for options
From: Jonathan Toppins @ 2022-05-17 20:31 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
Add support for reporting errors via extack in both bond_newlink
and bond_changelink.
Instead of having to look in the kernel log for why an option was not
correct just report the error to the user via the extack variable.
What is currently reported today:
ip link add bond0 type bond
ip link set bond0 up
ip link set bond0 type bond mode 4
RTNETLINK answers: Device or resource busy
After this change:
ip link add bond0 type bond
ip link set bond0 up
ip link set bond0 type bond mode 4
Error: option mode: unable to set because the bond device is up.
Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>
---
Notes:
This is an RFC because the current NL_SET_ERR_MSG() macros do not support
printf like semantics so I rolled my own buffer setting in __bond_opt_set().
The issue is I could not quite figure out the life-cycle of the buffer, if
rtnl lock is held until after the text buffer is copied into the packet
then we are ok, otherwise, some other type of buffer management scheme will
be needed as this could result in corrupted error messages when modifying
multiple bonds.
drivers/net/bonding/bond_netlink.c | 87 ++++++++++++++++++------------
drivers/net/bonding/bond_options.c | 62 +++++++++++++--------
include/net/bond_options.h | 2 +-
3 files changed, 96 insertions(+), 55 deletions(-)
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index f427fa1737c7..418a4f3d00a3 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -151,7 +151,7 @@ static int bond_slave_changelink(struct net_device *bond_dev,
snprintf(queue_id_str, sizeof(queue_id_str), "%s:%u\n",
slave_dev->name, queue_id);
bond_opt_initstr(&newval, queue_id_str);
- err = __bond_opt_set(bond, BOND_OPT_QUEUE_ID, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_QUEUE_ID, &newval, extack);
if (err)
return err;
}
@@ -175,7 +175,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int mode = nla_get_u8(data[IFLA_BOND_MODE]);
bond_opt_initval(&newval, mode);
- err = __bond_opt_set(bond, BOND_OPT_MODE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_MODE, &newval, extack);
if (err)
return err;
}
@@ -192,7 +192,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
active_slave = slave_dev->name;
}
bond_opt_initstr(&newval, active_slave);
- err = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval,
+ extack);
if (err)
return err;
}
@@ -200,7 +201,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
miimon = nla_get_u32(data[IFLA_BOND_MIIMON]);
bond_opt_initval(&newval, miimon);
- err = __bond_opt_set(bond, BOND_OPT_MIIMON, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_MIIMON, &newval, extack);
if (err)
return err;
}
@@ -208,7 +209,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int updelay = nla_get_u32(data[IFLA_BOND_UPDELAY]);
bond_opt_initval(&newval, updelay);
- err = __bond_opt_set(bond, BOND_OPT_UPDELAY, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_UPDELAY, &newval, extack);
if (err)
return err;
}
@@ -216,7 +217,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int downdelay = nla_get_u32(data[IFLA_BOND_DOWNDELAY]);
bond_opt_initval(&newval, downdelay);
- err = __bond_opt_set(bond, BOND_OPT_DOWNDELAY, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_DOWNDELAY, &newval, extack);
if (err)
return err;
}
@@ -224,7 +225,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int delay = nla_get_u32(data[IFLA_BOND_PEER_NOTIF_DELAY]);
bond_opt_initval(&newval, delay);
- err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval,
+ extack);
if (err)
return err;
}
@@ -232,7 +234,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int use_carrier = nla_get_u8(data[IFLA_BOND_USE_CARRIER]);
bond_opt_initval(&newval, use_carrier);
- err = __bond_opt_set(bond, BOND_OPT_USE_CARRIER, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_USE_CARRIER, &newval,
+ extack);
if (err)
return err;
}
@@ -240,12 +243,14 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int arp_interval = nla_get_u32(data[IFLA_BOND_ARP_INTERVAL]);
if (arp_interval && miimon) {
- netdev_err(bond->dev, "ARP monitoring cannot be used with MII monitoring\n");
+ NL_SET_ERR_MSG(extack,
+ "ARP monitoring cannot be used with MII monitoring");
return -EINVAL;
}
bond_opt_initval(&newval, arp_interval);
- err = __bond_opt_set(bond, BOND_OPT_ARP_INTERVAL, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_ARP_INTERVAL, &newval,
+ extack);
if (err)
return err;
}
@@ -264,7 +269,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
bond_opt_initval(&newval, (__force u64)target);
err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
- &newval);
+ &newval, extack);
if (err)
break;
i++;
@@ -297,7 +302,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
bond_opt_initextra(&newval, &addr6, sizeof(addr6));
err = __bond_opt_set(bond, BOND_OPT_NS_TARGETS,
- &newval);
+ &newval, extack);
if (err)
break;
i++;
@@ -312,12 +317,14 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int arp_validate = nla_get_u32(data[IFLA_BOND_ARP_VALIDATE]);
if (arp_validate && miimon) {
- netdev_err(bond->dev, "ARP validating cannot be used with MII monitoring\n");
+ NL_SET_ERR_MSG(extack,
+ "ARP validating cannot be used with MII monitoring");
return -EINVAL;
}
bond_opt_initval(&newval, arp_validate);
- err = __bond_opt_set(bond, BOND_OPT_ARP_VALIDATE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_ARP_VALIDATE, &newval,
+ extack);
if (err)
return err;
}
@@ -326,7 +333,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u32(data[IFLA_BOND_ARP_ALL_TARGETS]);
bond_opt_initval(&newval, arp_all_targets);
- err = __bond_opt_set(bond, BOND_OPT_ARP_ALL_TARGETS, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_ARP_ALL_TARGETS, &newval,
+ extack);
if (err)
return err;
}
@@ -340,7 +348,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
primary = dev->name;
bond_opt_initstr(&newval, primary);
- err = __bond_opt_set(bond, BOND_OPT_PRIMARY, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_PRIMARY, &newval, extack);
if (err)
return err;
}
@@ -349,7 +357,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_PRIMARY_RESELECT]);
bond_opt_initval(&newval, primary_reselect);
- err = __bond_opt_set(bond, BOND_OPT_PRIMARY_RESELECT, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_PRIMARY_RESELECT, &newval,
+ extack);
if (err)
return err;
}
@@ -358,7 +367,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_FAIL_OVER_MAC]);
bond_opt_initval(&newval, fail_over_mac);
- err = __bond_opt_set(bond, BOND_OPT_FAIL_OVER_MAC, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_FAIL_OVER_MAC, &newval,
+ extack);
if (err)
return err;
}
@@ -367,7 +377,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_XMIT_HASH_POLICY]);
bond_opt_initval(&newval, xmit_hash_policy);
- err = __bond_opt_set(bond, BOND_OPT_XMIT_HASH, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_XMIT_HASH, &newval, extack);
if (err)
return err;
}
@@ -376,7 +386,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u32(data[IFLA_BOND_RESEND_IGMP]);
bond_opt_initval(&newval, resend_igmp);
- err = __bond_opt_set(bond, BOND_OPT_RESEND_IGMP, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_RESEND_IGMP, &newval,
+ extack);
if (err)
return err;
}
@@ -385,7 +396,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_NUM_PEER_NOTIF]);
bond_opt_initval(&newval, num_peer_notif);
- err = __bond_opt_set(bond, BOND_OPT_NUM_PEER_NOTIF, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_NUM_PEER_NOTIF, &newval,
+ extack);
if (err)
return err;
}
@@ -394,7 +406,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_ALL_SLAVES_ACTIVE]);
bond_opt_initval(&newval, all_slaves_active);
- err = __bond_opt_set(bond, BOND_OPT_ALL_SLAVES_ACTIVE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_ALL_SLAVES_ACTIVE, &newval,
+ extack);
if (err)
return err;
}
@@ -403,7 +416,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u32(data[IFLA_BOND_MIN_LINKS]);
bond_opt_initval(&newval, min_links);
- err = __bond_opt_set(bond, BOND_OPT_MINLINKS, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_MINLINKS, &newval, extack);
if (err)
return err;
}
@@ -412,7 +425,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u32(data[IFLA_BOND_LP_INTERVAL]);
bond_opt_initval(&newval, lp_interval);
- err = __bond_opt_set(bond, BOND_OPT_LP_INTERVAL, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_LP_INTERVAL, &newval,
+ extack);
if (err)
return err;
}
@@ -421,7 +435,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u32(data[IFLA_BOND_PACKETS_PER_SLAVE]);
bond_opt_initval(&newval, packets_per_slave);
- err = __bond_opt_set(bond, BOND_OPT_PACKETS_PER_SLAVE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_PACKETS_PER_SLAVE, &newval,
+ extack);
if (err)
return err;
}
@@ -430,7 +445,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int lacp_active = nla_get_u8(data[IFLA_BOND_AD_LACP_ACTIVE]);
bond_opt_initval(&newval, lacp_active);
- err = __bond_opt_set(bond, BOND_OPT_LACP_ACTIVE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_LACP_ACTIVE, &newval,
+ extack);
if (err)
return err;
}
@@ -440,7 +456,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_AD_LACP_RATE]);
bond_opt_initval(&newval, lacp_rate);
- err = __bond_opt_set(bond, BOND_OPT_LACP_RATE, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_LACP_RATE, &newval, extack);
if (err)
return err;
}
@@ -449,7 +465,7 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u8(data[IFLA_BOND_AD_SELECT]);
bond_opt_initval(&newval, ad_select);
- err = __bond_opt_set(bond, BOND_OPT_AD_SELECT, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_AD_SELECT, &newval, extack);
if (err)
return err;
}
@@ -458,7 +474,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u16(data[IFLA_BOND_AD_ACTOR_SYS_PRIO]);
bond_opt_initval(&newval, actor_sys_prio);
- err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYS_PRIO, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYS_PRIO, &newval,
+ extack);
if (err)
return err;
}
@@ -467,7 +484,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
nla_get_u16(data[IFLA_BOND_AD_USER_PORT_KEY]);
bond_opt_initval(&newval, port_key);
- err = __bond_opt_set(bond, BOND_OPT_AD_USER_PORT_KEY, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_AD_USER_PORT_KEY, &newval,
+ extack);
if (err)
return err;
}
@@ -477,7 +495,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
bond_opt_initval(&newval,
nla_get_u64(data[IFLA_BOND_AD_ACTOR_SYSTEM]));
- err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYSTEM, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYSTEM, &newval,
+ extack);
if (err)
return err;
}
@@ -485,7 +504,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int dynamic_lb = nla_get_u8(data[IFLA_BOND_TLB_DYNAMIC_LB]);
bond_opt_initval(&newval, dynamic_lb);
- err = __bond_opt_set(bond, BOND_OPT_TLB_DYNAMIC_LB, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_TLB_DYNAMIC_LB, &newval,
+ extack);
if (err)
return err;
}
@@ -494,7 +514,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
int missed_max = nla_get_u8(data[IFLA_BOND_MISSED_MAX]);
bond_opt_initval(&newval, missed_max);
- err = __bond_opt_set(bond, BOND_OPT_MISSED_MAX, &newval);
+ err = __bond_opt_set(bond, BOND_OPT_MISSED_MAX, &newval,
+ extack);
if (err)
return err;
}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 64f7db2627ce..4a95503384a3 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -636,7 +636,8 @@ static int bond_opt_check_deps(struct bonding *bond,
}
static void bond_opt_dep_print(struct bonding *bond,
- const struct bond_option *opt)
+ const struct bond_option *opt,
+ char *buf, size_t bufsize)
{
const struct bond_opt_value *modeval;
struct bond_params *params;
@@ -644,16 +645,18 @@ static void bond_opt_dep_print(struct bonding *bond,
params = &bond->params;
modeval = bond_opt_get_val(BOND_OPT_MODE, params->mode);
if (test_bit(params->mode, &opt->unsuppmodes))
- netdev_err(bond->dev, "option %s: mode dependency failed, not supported in mode %s(%llu)\n",
- opt->name, modeval->string, modeval->value);
+ scnprintf(buf, bufsize, "option %s: mode dependency failed, not supported in mode %s(%llu)\n",
+ opt->name, modeval->string, modeval->value);
}
static void bond_opt_error_interpret(struct bonding *bond,
const struct bond_option *opt,
- int error, const struct bond_opt_value *val)
+ int error, const struct bond_opt_value *val,
+ char *buf, size_t bufsize)
{
const struct bond_opt_value *minval, *maxval;
char *p;
+ int i = 0;
switch (error) {
case -EINVAL:
@@ -663,38 +666,45 @@ static void bond_opt_error_interpret(struct bonding *bond,
p = strchr(val->string, '\n');
if (p)
*p = '\0';
- netdev_err(bond->dev, "option %s: invalid value (%s)\n",
- opt->name, val->string);
+ i = scnprintf(buf, bufsize,
+ "option %s: invalid value (%s)",
+ opt->name, val->string);
} else {
- netdev_err(bond->dev, "option %s: invalid value (%llu)\n",
- opt->name, val->value);
+ i = scnprintf(buf, bufsize,
+ "option %s: invalid value (%llu)",
+ opt->name, val->value);
}
}
minval = bond_opt_get_flags(opt, BOND_VALFLAG_MIN);
maxval = bond_opt_get_flags(opt, BOND_VALFLAG_MAX);
if (!maxval)
break;
- netdev_err(bond->dev, "option %s: allowed values %llu - %llu\n",
- opt->name, minval ? minval->value : 0, maxval->value);
+ if (i) {
+ // index buf to overwirte '\n' from above
+ buf = &buf[i];
+ bufsize -= i;
+ }
+ scnprintf(buf, bufsize, " allowed values %llu - %llu",
+ minval ? minval->value : 0, maxval->value);
break;
case -EACCES:
- bond_opt_dep_print(bond, opt);
+ bond_opt_dep_print(bond, opt, buf, bufsize);
break;
case -ENOTEMPTY:
- netdev_err(bond->dev, "option %s: unable to set because the bond device has slaves\n",
- opt->name);
+ scnprintf(buf, bufsize, "option %s: unable to set because the bond device has slaves",
+ opt->name);
break;
case -EBUSY:
- netdev_err(bond->dev, "option %s: unable to set because the bond device is up\n",
- opt->name);
+ scnprintf(buf, bufsize, "option %s: unable to set because the bond device is up",
+ opt->name);
break;
case -ENODEV:
if (val && val->string) {
p = strchr(val->string, '\n');
if (p)
*p = '\0';
- netdev_err(bond->dev, "option %s: interface %s does not exist!\n",
- opt->name, val->string);
+ scnprintf(buf, bufsize, "option %s: interface %s does not exist!",
+ opt->name, val->string);
}
break;
default:
@@ -713,7 +723,8 @@ static void bond_opt_error_interpret(struct bonding *bond,
* must be obtained before calling this function.
*/
int __bond_opt_set(struct bonding *bond,
- unsigned int option, struct bond_opt_value *val)
+ unsigned int option, struct bond_opt_value *val,
+ struct netlink_ext_ack *extack)
{
const struct bond_opt_value *retval = NULL;
const struct bond_option *opt;
@@ -734,8 +745,17 @@ int __bond_opt_set(struct bonding *bond,
}
ret = opt->set(bond, retval);
out:
- if (ret)
- bond_opt_error_interpret(bond, opt, ret, val);
+ if (ret) {
+ static char buf[120];
+ buf[0] = '\0';
+ bond_opt_error_interpret(bond, opt, ret, val, buf, sizeof(buf));
+ if (buf[0] != '\0') {
+ if (extack)
+ extack->_msg = buf;
+ else
+ netdev_err(bond->dev, "Error: %s\n", buf);
+ }
+ }
return ret;
}
@@ -757,7 +777,7 @@ int __bond_opt_set_notify(struct bonding *bond,
ASSERT_RTNL();
- ret = __bond_opt_set(bond, option, val);
+ ret = __bond_opt_set(bond, option, val, NULL);
if (!ret && (bond->dev->reg_state == NETREG_REGISTERED))
call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 61b49063791c..ae38557adc25 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -107,7 +107,7 @@ struct bond_option {
};
int __bond_opt_set(struct bonding *bond, unsigned int option,
- struct bond_opt_value *val);
+ struct bond_opt_value *val, struct netlink_ext_ack *extack);
int __bond_opt_set_notify(struct bonding *bond, unsigned int option,
struct bond_opt_value *val);
int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf);
--
2.27.0
^ permalink raw reply related
* Re: [PATCH net] tcp: Add READ_ONCE() to read tcp_orphan_count
From: Paul E. McKenney @ 2022-05-17 20:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Marco Elver, Liu Jian, Dmitry Vyukov, LKML, David Miller,
Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski, Paolo Abeni,
Neal Cardwell, netdev
In-Reply-To: <CANn89iKiTiGwMvV6K+Zbr_9+knaR-x1N3tOeMX+2No2=4zn6pA@mail.gmail.com>
On Thu, May 12, 2022 at 04:43:20PM -0700, Eric Dumazet wrote:
> On Thu, May 12, 2022 at 4:10 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, May 12, 2022 at 02:31:48PM -0700, Eric Dumazet wrote:
> > > On Thu, May 12, 2022 at 2:18 PM Marco Elver <elver@google.com> wrote:
> > >
> > > >
> > > > I guess the question is, is it the norm that per_cpu() retrieves data
> > > > that can legally be modified concurrently, or not. If not, and in most
> > > > cases it's a bug, the annotations should be here.
> > > >
> > > > Paul, was there any guidance/documentation on this, but I fail to find
> > > > it right now? (access-marking.txt doesn't say much about per-CPU
> > > > data.)
> > >
> > > Normally, whenever we add a READ_ONCE(), we are supposed to add a comment.
> >
> > I am starting to think that comments are even more necessary for unmarked
> > accesses to shared variables, with the comments setting out why the
> > compiler cannot mess things up. ;-)
> >
> > > We could make an exception for per_cpu_once(), because the comment
> > > would be centralized
> > > at per_cpu_once() definition.
> >
> > This makes a lot of sense to me.
> >
> > > We will be stuck with READ_ONCE() in places we are using
> > > per_cpu_ptr(), for example
> > > in dev_fetch_sw_netstats()
> >
> > If this is strictly statistics, data_race() is another possibility.
> > But it does not constrain the compiler at all.
>
> Statistics are supposed to be monotonically increasing ;)
>
> Some SNMP agents would be very confused if they could observe 'garbage' there.
>
> I sense that we are going to add thousands of READ_ONCE() soon :/
Indeed, adding READ_ONCE() instances can be annoying. Then again, it
can also be annoying to have to debug the problems that sometimes arise
from omitting them where they are needed.
Thanx, Paul
^ permalink raw reply
* [syzbot] KASAN: slab-out-of-bounds Read in cttimeout_net_exit
From: syzbot @ 2022-05-17 20:27 UTC (permalink / raw)
To: coreteam, davem, edumazet, fw, kadlec, kuba, linux-kernel, netdev,
netfilter-devel, pabeni, pablo, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 65a9dedc11d6 net: phy: marvell: Add errata section 5.1 for..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=16fafa9ef00000
kernel config: https://syzkaller.appspot.com/x/.config?x=c05eee2efc702eed
dashboard link: https://syzkaller.appspot.com/bug?extid=92968395eedbdbd3617d
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+92968395eedbdbd3617d@syzkaller.appspotmail.com
bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
bond0 (unregistering): (slave bond_slave_0): Releasing backup interface
bond0 (unregistering): Released all slaves
==================================================================
BUG: KASAN: slab-out-of-bounds in __list_del_entry_valid+0xcc/0xf0 lib/list_debug.c:42
Read of size 8 at addr ffff888051af75b8 by task kworker/u4:5/1223
CPU: 1 PID: 1223 Comm: kworker/u4:5 Not tainted 5.18.0-rc6-syzkaller-01553-g65a9dedc11d6 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: netns cleanup_net
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description.constprop.0.cold+0xeb/0x495 mm/kasan/report.c:313
print_report mm/kasan/report.c:429 [inline]
kasan_report.cold+0xf4/0x1c6 mm/kasan/report.c:491
__list_del_entry_valid+0xcc/0xf0 lib/list_debug.c:42
__list_del_entry include/linux/list.h:134 [inline]
list_del include/linux/list.h:148 [inline]
cttimeout_net_exit+0x211/0x540 net/netfilter/nfnetlink_cttimeout.c:617
ops_exit_list+0xb0/0x170 net/core/net_namespace.c:162
cleanup_net+0x4ea/0xb00 net/core/net_namespace.c:594
process_one_work+0x996/0x1610 kernel/workqueue.c:2289
worker_thread+0x665/0x1080 kernel/workqueue.c:2436
kthread+0x2e9/0x3a0 kernel/kthread.c:376
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298
</TASK>
Allocated by task 9270:
kasan_save_stack+0x1e/0x40 mm/kasan/common.c:38
kasan_set_track mm/kasan/common.c:45 [inline]
set_alloc_info mm/kasan/common.c:436 [inline]
____kasan_kmalloc mm/kasan/common.c:515 [inline]
____kasan_kmalloc mm/kasan/common.c:474 [inline]
__kasan_kmalloc+0xa9/0xd0 mm/kasan/common.c:524
kmalloc include/linux/slab.h:586 [inline]
kzalloc include/linux/slab.h:714 [inline]
cttimeout_new_timeout+0x51f/0xa50 net/netfilter/nfnetlink_cttimeout.c:156
nfnetlink_rcv_msg+0xbcd/0x13f0 net/netfilter/nfnetlink.c:297
netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2501
nfnetlink_rcv+0x1ac/0x420 net/netfilter/nfnetlink.c:655
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x543/0x7f0 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x917/0xe10 net/netlink/af_netlink.c:1921
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:734
____sys_sendmsg+0x6eb/0x810 net/socket.c:2460
___sys_sendmsg+0xf3/0x170 net/socket.c:2514
__sys_sendmsg net/socket.c:2543 [inline]
__do_sys_sendmsg net/socket.c:2552 [inline]
__se_sys_sendmsg net/socket.c:2550 [inline]
__x64_sys_sendmsg+0x132/0x220 net/socket.c:2550
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
The buggy address belongs to the object at ffff888051af7500
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 56 bytes to the right of
128-byte region [ffff888051af7500, ffff888051af7580)
The buggy address belongs to the physical page:
page:ffffea000146bdc0 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888051af7100 pfn:0x51af7
flags: 0xfff00000000200(slab|node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000000200 ffffea0001ed5e48 ffffea0001cf4b08 ffff888010c418c0
raw: ffff888051af7100 000000000010000b 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x112cc0(GFP_USER|__GFP_NOWARN|__GFP_NORETRY), pid 7027, tgid 7027 (cmp), ts 227162348638, free_ts 227160439771
prep_new_page mm/page_alloc.c:2441 [inline]
get_page_from_freelist+0xba2/0x3e00 mm/page_alloc.c:4182
__alloc_pages+0x1b2/0x500 mm/page_alloc.c:5408
__alloc_pages_node include/linux/gfp.h:587 [inline]
alloc_slab_page mm/slub.c:1801 [inline]
allocate_slab+0x80/0x3c0 mm/slub.c:1944
new_slab mm/slub.c:2004 [inline]
___slab_alloc+0x8df/0xf20 mm/slub.c:3005
__slab_alloc.constprop.0+0x4d/0xa0 mm/slub.c:3092
slab_alloc_node mm/slub.c:3183 [inline]
__kmalloc_node+0x2cb/0x390 mm/slub.c:4458
kmalloc_array_node include/linux/slab.h:676 [inline]
kcalloc_node include/linux/slab.h:681 [inline]
memcg_alloc_slab_cgroups+0x8b/0x140 mm/memcontrol.c:2810
account_slab mm/slab.h:652 [inline]
allocate_slab+0x2c9/0x3c0 mm/slub.c:1960
new_slab mm/slub.c:2004 [inline]
___slab_alloc+0x8df/0xf20 mm/slub.c:3005
__slab_alloc.constprop.0+0x4d/0xa0 mm/slub.c:3092
slab_alloc_node mm/slub.c:3183 [inline]
slab_alloc mm/slub.c:3225 [inline]
__kmem_cache_alloc_lru mm/slub.c:3232 [inline]
kmem_cache_alloc+0x360/0x3b0 mm/slub.c:3242
vm_area_dup+0x88/0x3f0 kernel/fork.c:467
__split_vma+0xa5/0x550 mm/mmap.c:2712
split_vma+0x95/0xd0 mm/mmap.c:2770
mprotect_fixup+0x72d/0x950 mm/mprotect.c:494
do_mprotect_pkey+0x532/0x980 mm/mprotect.c:650
page last free stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline]
free_pages_prepare mm/page_alloc.c:1356 [inline]
free_pcp_prepare+0x549/0xd20 mm/page_alloc.c:1406
free_unref_page_prepare mm/page_alloc.c:3328 [inline]
free_unref_page+0x19/0x6a0 mm/page_alloc.c:3423
__vunmap+0x85d/0xd30 mm/vmalloc.c:2667
free_work+0x58/0x70 mm/vmalloc.c:97
process_one_work+0x996/0x1610 kernel/workqueue.c:2289
worker_thread+0x665/0x1080 kernel/workqueue.c:2436
kthread+0x2e9/0x3a0 kernel/kthread.c:376
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298
Memory state around the buggy address:
ffff888051af7480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888051af7500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fc fc
>ffff888051af7580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff888051af7600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888051af7680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* Re: [PATCH 2/2] octeon_ep: Fix irq releasing in the error handling path of octep_request_irqs()
From: Christophe JAILLET @ 2022-05-17 20:12 UTC (permalink / raw)
To: Paolo Abeni, Dan Carpenter
Cc: Veerasenareddy Burru, Abhijit Ayarekar, David S. Miller,
Eric Dumazet, Jakub Kicinski, Satananda Burla, linux-kernel,
kernel-janitors, netdev
In-Reply-To: <eec880be771e75d60ead01cbf71d83fe070ccde8.camel@redhat.com>
Le 17/05/2022 à 10:35, Paolo Abeni a écrit :
> On Tue, 2022-05-17 at 08:28 +0300, Dan Carpenter wrote:
>> On Sun, May 15, 2022 at 05:56:45PM +0200, Christophe JAILLET wrote:
>>> For the error handling to work as expected, the index in the
>>> 'oct->msix_entries' array must be tweaked because, when the irq are
>>> requested there is:
>>> msix_entry = &oct->msix_entries[i + num_non_ioq_msix];
>>>
>>> So in the error handling path, 'i + num_non_ioq_msix' should be used
>>> instead of 'i'.
>>>
>>> The 2nd argument of free_irq() also needs to be adjusted.
>>>
>>> Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>> ---
>>> I think that the wording above is awful, but I'm sure you get it.
>>> Feel free to rephrase everything to have it more readable.
>>> ---
>>> drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
>>> index 6b60a03574a0..4dcae805422b 100644
>>> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
>>> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
>>> @@ -257,10 +257,12 @@ static int octep_request_irqs(struct octep_device *oct)
>>>
>>> return 0;
>>> ioq_irq_err:
>>> + i += num_non_ioq_msix;
>>> while (i > num_non_ioq_msix) {
>>
>> This makes my mind hurt so badly. Can we not just have two variables
>> for the two different loops instead of re-using i?
>>
>>> --i;
>>> irq_set_affinity_hint(oct->msix_entries[i].vector, NULL);
>>> - free_irq(oct->msix_entries[i].vector, oct->ioq_vector[i]);
>>> + free_irq(oct->msix_entries[i].vector,
>>> + oct->ioq_vector[i - num_non_ioq_msix]);
>>> }
>>
>> ioq_irq_err:
>> while (--j >= 0) {
>> ioq_vector = oct->ioq_vector[j];
>> msix_entry = &oct->msix_entries[j + num_non_ioq_msix];
>>
>> irq_set_affinity_hint(msix_entry->vector, NULL);
>> free_irq(msix_entry->vector, ioq_vector);
>> }
>>
>> regards,
>> dan carpenter
>
> I agree the above would be more readable. @Christophe: could you please
> refactor the code as per Dan's suggestion?
Will do.
I was sure that Dan would comment on this unusual pattern :)
CJ
>
> Thanks!
>
> Paolo
>
>
^ permalink raw reply
* RE: [PATCH 05/12] net: mana: Set the DMA device max page size
From: Long Li @ 2022-05-17 20:04 UTC (permalink / raw)
To: Jason Gunthorpe, Ajay Sharma
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, David S. Miller, Jakub Kicinski, Paolo Abeni,
Leon Romanovsky, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20220517193515.GN63055@ziepe.ca>
> Subject: Re: [PATCH 05/12] net: mana: Set the DMA device max page size
>
> On Tue, May 17, 2022 at 07:32:51PM +0000, Long Li wrote:
> > > Subject: Re: [PATCH 05/12] net: mana: Set the DMA device max page
> > > size
> > >
> > > On Tue, May 17, 2022 at 02:04:29AM -0700, longli@linuxonhyperv.com
> wrote:
> > > > From: Long Li <longli@microsoft.com>
> > > >
> > > > The system chooses default 64K page size if the device does not
> > > > specify the max page size the device can handle for DMA. This do
> > > > not work well when device is registering large chunk of memory in
> > > > that a large page size is more efficient.
> > > >
> > > > Set it to the maximum hardware supported page size.
> > >
> > > For RDMA devices this should be set to the largest segment size an
> > > ib_sge can take in when posting work. It should not be the page size
> > > of MR. 2M is a weird number for that, are you sure it is right?
> >
> > Yes, this is the maximum page size used in hardware page tables.
>
> As I said, it should be the size of the sge in the WQE, not the "hardware page
> tables"
This driver uses the following code to figure out the largest page size for memory registration with hardware:
page_sz = ib_umem_find_best_pgsz(mr->umem, PAGE_SZ_BM, iova);
In this function, mr->umem is created with ib_dma_max_seg_size() as its max segment size when creating its sgtable.
The purpose of setting DMA page size to 2M is to make sure this function returns the largest possible MR size that the hardware can take. Otherwise, this function will return 64k: the default DMA size.
Long
>
> Jason
^ permalink raw reply
* Re: [PATCH bpf-next v3 4/4] bpf_trace: pass array of u64 values in kprobe_multi.addrs
From: Jiri Olsa @ 2022-05-17 20:03 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: Jiri Olsa, Masami Hiramatsu, Steven Rostedt, Ingo Molnar,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, netdev, bpf, linux-kernel, Shuah Khan, linux-kselftest
In-Reply-To: <20220517123050.GA25149@asgard.redhat.com>
On Tue, May 17, 2022 at 02:30:50PM +0200, Eugene Syromiatnikov wrote:
> On Tue, May 17, 2022 at 11:12:34AM +0200, Jiri Olsa wrote:
> > On Tue, May 17, 2022 at 09:36:47AM +0200, Eugene Syromiatnikov wrote:
> > > With the interface as defined, it is impossible to pass 64-bit kernel
> > > addresses from a 32-bit userspace process in BPF_LINK_TYPE_KPROBE_MULTI,
> > > which severly limits the useability of the interface, change the ABI
> > > to accept an array of u64 values instead of (kernel? user?) longs.
> > > Interestingly, the rest of the libbpf infrastructure uses 64-bit values
> > > for kallsyms addresses already, so this patch also eliminates
> > > the sym_addr cast in tools/lib/bpf/libbpf.c:resolve_kprobe_multi_cb().
> >
> > so the problem is when we have 32bit user sace on 64bit kernel right?
> >
> > I think we should keep addrs as longs in uapi and have kernel to figure out
> > if it needs to read u32 or u64, like you did for symbols in previous patch
>
> No, it's not possible here, as addrs are kernel addrs and not user space
> addrs, so user space has to explicitly pass 64-bit addresses on 64-bit
> kernels (or have a notion whether it is running on a 64-bit
> or 32-bit kernel, and form the passed array accordingly, which is against
> the idea of compat layer that tries to abstract it out).
hum :-\ I'll need to check on compat layer.. there must
be some other code doing this already somewhere, right?
jirka
>
> > we'll need to fix also bpf_kprobe_multi_cookie_swap because it assumes
> > 64bit user space pointers
> >
> > would be gret if we could have selftest for this
> >
> > thanks,
> > jirka
> >
> > >
> > > Fixes: 0dcac272540613d4 ("bpf: Add multi kprobe link")
> > > Fixes: 5117c26e877352bc ("libbpf: Add bpf_link_create support for multi kprobes")
> > > Fixes: ddc6b04989eb0993 ("libbpf: Add bpf_program__attach_kprobe_multi_opts function")
> > > Fixes: f7a11eeccb111854 ("selftests/bpf: Add kprobe_multi attach test")
> > > Fixes: 9271a0c7ae7a9147 ("selftests/bpf: Add attach test for bpf_program__attach_kprobe_multi_opts")
> > > Fixes: 2c6401c966ae1fbe ("selftests/bpf: Add kprobe_multi bpf_cookie test")
> > > Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
> > > ---
> > > kernel/trace/bpf_trace.c | 25 ++++++++++++++++++----
> > > tools/lib/bpf/bpf.h | 2 +-
> > > tools/lib/bpf/libbpf.c | 8 +++----
> > > tools/lib/bpf/libbpf.h | 2 +-
> > > .../testing/selftests/bpf/prog_tests/bpf_cookie.c | 2 +-
> > > .../selftests/bpf/prog_tests/kprobe_multi_test.c | 8 +++----
> > > 6 files changed, 32 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > index 9d3028a..30a15b3 100644
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -2454,7 +2454,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> > > void __user *ucookies;
> > > unsigned long *addrs;
> > > u32 flags, cnt, size, cookies_size;
> > > - void __user *uaddrs;
> > > + u64 __user *uaddrs;
> > > u64 *cookies = NULL;
> > > void __user *usyms;
> > > int err;
> > > @@ -2486,9 +2486,26 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> > > return -ENOMEM;
> > >
> > > if (uaddrs) {
> > > - if (copy_from_user(addrs, uaddrs, size)) {
> > > - err = -EFAULT;
> > > - goto error;
> > > + if (sizeof(*addrs) == sizeof(*uaddrs)) {
> > > + if (copy_from_user(addrs, uaddrs, size)) {
> > > + err = -EFAULT;
> > > + goto error;
> > > + }
> > > + } else {
> > > + u32 i;
> > > + u64 addr;
> > > +
> > > + for (i = 0; i < cnt; i++) {
> > > + if (get_user(addr, uaddrs + i)) {
> > > + err = -EFAULT;
> > > + goto error;
> > > + }
> > > + if (addr > ULONG_MAX) {
> > > + err = -EINVAL;
> > > + goto error;
> > > + }
> > > + addrs[i] = addr;
> > > + }
> > > }
> > > } else {
> > > struct user_syms us;
> > > diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> > > index 2e0d373..da9c6037 100644
> > > --- a/tools/lib/bpf/bpf.h
> > > +++ b/tools/lib/bpf/bpf.h
> > > @@ -418,7 +418,7 @@ struct bpf_link_create_opts {
> > > __u32 flags;
> > > __u32 cnt;
> > > const char **syms;
> > > - const unsigned long *addrs;
> > > + const __u64 *addrs;
> > > const __u64 *cookies;
> > > } kprobe_multi;
> > > struct {
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index ef7f302..35fa9c5 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -10737,7 +10737,7 @@ static bool glob_match(const char *str, const char *pat)
> > >
> > > struct kprobe_multi_resolve {
> > > const char *pattern;
> > > - unsigned long *addrs;
> > > + __u64 *addrs;
> > > size_t cap;
> > > size_t cnt;
> > > };
> > > @@ -10752,12 +10752,12 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type,
> > > if (!glob_match(sym_name, res->pattern))
> > > return 0;
> > >
> > > - err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long),
> > > + err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(__u64),
> > > res->cnt + 1);
> > > if (err)
> > > return err;
> > >
> > > - res->addrs[res->cnt++] = (unsigned long) sym_addr;
> > > + res->addrs[res->cnt++] = sym_addr;
> > > return 0;
> > > }
> > >
> > > @@ -10772,7 +10772,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> > > };
> > > struct bpf_link *link = NULL;
> > > char errmsg[STRERR_BUFSIZE];
> > > - const unsigned long *addrs;
> > > + const __u64 *addrs;
> > > int err, link_fd, prog_fd;
> > > const __u64 *cookies;
> > > const char **syms;
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index 9e9a3fd..76e171d 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -489,7 +489,7 @@ struct bpf_kprobe_multi_opts {
> > > /* array of function symbols to attach */
> > > const char **syms;
> > > /* array of function addresses to attach */
> > > - const unsigned long *addrs;
> > > + const __u64 *addrs;
> > > /* array of user-provided values fetchable through bpf_get_attach_cookie */
> > > const __u64 *cookies;
> > > /* number of elements in syms/addrs/cookies arrays */
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > > index 83ef55e3..e843840 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> > > @@ -140,7 +140,7 @@ static void kprobe_multi_link_api_subtest(void)
> > > cookies[6] = 7;
> > > cookies[7] = 8;
> > >
> > > - opts.kprobe_multi.addrs = (const unsigned long *) &addrs;
> > > + opts.kprobe_multi.addrs = (const __u64 *) &addrs;
> > > opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> > > opts.kprobe_multi.cookies = (const __u64 *) &cookies;
> > > prog_fd = bpf_program__fd(skel->progs.test_kprobe);
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > > index 586dc52..7646112 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > > @@ -108,7 +108,7 @@ static void test_link_api_addrs(void)
> > > GET_ADDR("bpf_fentry_test7", addrs[6]);
> > > GET_ADDR("bpf_fentry_test8", addrs[7]);
> > >
> > > - opts.kprobe_multi.addrs = (const unsigned long*) addrs;
> > > + opts.kprobe_multi.addrs = (const __u64 *) addrs;
> > > opts.kprobe_multi.cnt = ARRAY_SIZE(addrs);
> > > test_link_api(&opts);
> > > }
> > > @@ -186,7 +186,7 @@ static void test_attach_api_addrs(void)
> > > GET_ADDR("bpf_fentry_test7", addrs[6]);
> > > GET_ADDR("bpf_fentry_test8", addrs[7]);
> > >
> > > - opts.addrs = (const unsigned long *) addrs;
> > > + opts.addrs = (const __u64 *) addrs;
> > > opts.cnt = ARRAY_SIZE(addrs);
> > > test_attach_api(NULL, &opts);
> > > }
> > > @@ -244,7 +244,7 @@ static void test_attach_api_fails(void)
> > > goto cleanup;
> > >
> > > /* fail_2 - both addrs and syms set */
> > > - opts.addrs = (const unsigned long *) addrs;
> > > + opts.addrs = (const __u64 *) addrs;
> > > opts.syms = syms;
> > > opts.cnt = ARRAY_SIZE(syms);
> > > opts.cookies = NULL;
> > > @@ -258,7 +258,7 @@ static void test_attach_api_fails(void)
> > > goto cleanup;
> > >
> > > /* fail_3 - pattern and addrs set */
> > > - opts.addrs = (const unsigned long *) addrs;
> > > + opts.addrs = (const __u64 *) addrs;
> > > opts.syms = NULL;
> > > opts.cnt = ARRAY_SIZE(syms);
> > > opts.cookies = NULL;
> > > --
> > > 2.1.4
> > >
> >
>
^ permalink raw reply
* Re: [PATCH net-next] net: ifdefy the wireless pointers in struct net_device
From: Jakub Kicinski @ 2022-05-17 19:49 UTC (permalink / raw)
To: Alexander Aring
Cc: Florian Fainelli, David S. Miller, Network Development, edumazet,
Paolo Abeni, johannes, Alexander Aring, Stefan Schmidt,
mareklindner, sw, a, sven, linux-wireless, linux-wpan - ML
In-Reply-To: <CAK-6q+jRDMDGbNS2JkTXmW2dp6D7mGzZ6ghrjf7m-wp7Xo9weQ@mail.gmail.com>
On Tue, 17 May 2022 15:33:02 -0400 Alexander Aring wrote:
> > Could not we move to an union of pointers in the future since in many
> > cases a network device can only have one of those pointers at any given
> > time?
>
> note that ieee802154 has also functionality like __dev_get_by_index()
> and checks via "if (netdev->ieee802154_ptr)" if it's a wpan interface
> or not, guess the solution would be like it's done in wireless then.
Ack, but that code must live somewhere under
#if IS_ENABLED(CONFIG_IEEE802154) || IS_ENABLED(CONFIG_6LOWPAN)
otherwise I think I'd see a build failure. I guess a nice thing about
having the typed pointers is that we can depend on the compiler for
basic checking :)
^ permalink raw reply
* RE: [PATCH 09/12] net: mana: Move header files to a common location
From: Long Li @ 2022-05-17 19:34 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, David S. Miller, Jakub Kicinski, Paolo Abeni,
Leon Romanovsky, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <20220517150339.GI63055@ziepe.ca>
> Subject: Re: [PATCH 09/12] net: mana: Move header files to a common location
>
> On Tue, May 17, 2022 at 02:04:33AM -0700, longli@linuxonhyperv.com wrote:
> > From: Long Li <longli@microsoft.com>
> >
> > In preparation to add MANA RDMA driver, move all the required header
> > files to a common location for use by both Ethernet and RDMA drivers.
> >
> > Signed-off-by: Long Li <longli@microsoft.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/net/ethernet/microsoft/mana/gdma_main.c | 2 +-
> > drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 ++--
> > drivers/net/ethernet/microsoft/mana/mana_bpf.c | 2 +-
> > drivers/net/ethernet/microsoft/mana/mana_en.c | 2 +-
> > drivers/net/ethernet/microsoft/mana/mana_ethtool.c | 2 +-
> > drivers/net/ethernet/microsoft/mana/shm_channel.c | 2 +-
> > {drivers/net/ethernet/microsoft => include/linux}/mana/gdma.h | 0
> > .../ethernet/microsoft => include/linux}/mana/hw_channel.h | 0
> > {drivers/net/ethernet/microsoft => include/linux}/mana/mana.h | 0
> > .../ethernet/microsoft => include/linux}/mana/shm_channel.h | 0
> > 11 files changed, 8 insertions(+), 7 deletions(-) rename
> > {drivers/net/ethernet/microsoft => include/linux}/mana/gdma.h (100%)
> > rename {drivers/net/ethernet/microsoft =>
> > include/linux}/mana/hw_channel.h (100%) rename
> > {drivers/net/ethernet/microsoft => include/linux}/mana/mana.h (100%)
> > rename {drivers/net/ethernet/microsoft =>
> > include/linux}/mana/shm_channel.h (100%)
>
> I know mlx5 did it like this, but I wonder if include/net is more appropriate?
>
> Or maybe include/aux/?
I can move the header files to include/net/mana, if that sounds okay and no objection to doing that.
Long
>
> Jason
^ permalink raw reply
* Re: [syzbot] KASAN: use-after-free Read in nf_confirm
From: Florian Westphal @ 2022-05-17 19:47 UTC (permalink / raw)
To: syzbot
Cc: ali.abdallah, coreteam, davem, edumazet, fw, kadlec, kuba,
linux-kernel, netdev, netfilter-devel, ozsh, pabeni, pablo, paulb,
syzkaller-bugs
In-Reply-To: <000000000000fb4af305df391431@google.com>
syzbot <syzbot+793a590957d9c1b96620@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: d887ae3247e0 octeontx2-pf: Remove unnecessary synchronize_..
> git tree: net-next
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=17f2b659f00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=b1aab282dc5dd920
> dashboard link: https://syzkaller.appspot.com/bug?extid=793a590957d9c1b96620
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1313dce6f00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=169eb59ef00000
>
> The issue was bisected to:
>
> commit 1397af5bfd7d32b0cf2adb70a78c9a9e8f11d912
> Author: Florian Westphal <fw@strlen.de>
> Date: Mon Apr 11 11:01:18 2022 +0000
>
> netfilter: conntrack: remove the percpu dying list
AFAICS this bug exists since
Fixes: 71d8c47fc653 ("netfilter: conntrack: introduce clash resolution on insertion race")
nf_confirm needs to re-fetch 'ct' from skb->_nfct. Will send a patch.
^ permalink raw reply
* Re: [PATCH net 0/2] selftests: net: add missing tests to Makefile
From: Jakub Kicinski @ 2022-05-17 19:45 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, David S. Miller, Paolo Abeni, Shuah Khan, linux-kselftest
In-Reply-To: <YoM/Wr6FaTzgokx3@Laptop-X1>
On Tue, 17 May 2022 14:23:22 +0800 Hangbin Liu wrote:
> On Fri, Apr 29, 2022 at 05:56:04PM -0700, Jakub Kicinski wrote:
> > On Thu, 28 Apr 2022 12:45:09 +0800 Hangbin Liu wrote:
> > > I think there need a way to notify the developer when they created a new
> > > file in selftests folder. Maybe a bot like bluez.test.bot or kernel
> > > test robot could help do that?
> >
> > Our netdev patch checks are here:
> >
> > https://github.com/kuba-moo/nipa/tree/master/tests/patch
> >
> > in case you're willing to code it up and post a PR.
>
> Hi Jakub,
>
> I checked the tools and write a draft patch. But I have a question before post
> the PR.
First off - thanks a log for doing this!
> AFAIK, This bot is only used for checking patches and adding status in
> patchwork. But it doesn't support sending a reply to developer, right?
>
> For the selftest reminder, I think it would be good to let developer know
> via email if the file is missing in Makefile. What do you think?
Yes, we don't have the auto-reply. There's too much noise in some of
the tests, but mostly it's because we don't want to encourage people
posting patches just to build them. If it's a machine replying rather
than a human some may think that it's okay. We already have
jaw-droppingly expensive VM instance to keep up with the build volume.
And the list is very busy. So we can't afford "post to run the CI"
development model.
> Here is the draft patch:
>
> diff --git a/tests/patch/check_selftest/check_selftest.sh b/tests/patch/check_selftest/check_selftest.sh
> new file mode 100755
> index 0000000..ad7c608
> --- /dev/null
> +++ b/tests/patch/check_selftest/check_selftest.sh
> @@ -0,0 +1,28 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +rt=0
> +if ! git show --name-status --oneline | \
> + grep -P '^A\ttools/testing/selftests/net/' | \
> + grep '\.sh$'; then
> + echo "No new net selftests script" >&$DESC_FD
> + exit 0
> +fi
> +
> +files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '\.sh$' | sed 's@A\ttools/testing/selftests/net/@@')
> +for file in $files; do
> + if echo $file | grep forwarding; then
> + file=$(echo $file | sed 's/forwarding\///')
> + if ! grep -P "[\t| ]$file" tools/testing/selftests/net/forwarding/Makefile;then
> + echo "new test $file not in selftests/net/forwarding/Makefile" >&$DESC_FD
> + rc=1
> + fi
> + else
> + if ! grep -P "[\t| ]$file" tools/testing/selftests/net/Makefile;then
> + echo "new test $file not in selftests/net/Makefile" >&$DESC_FD
> + rc=1
> + fi
Does it matter which exact selftest makefile the changes are?
Maybe as a first stab we should just check if there are changes
to anything in tools/testing/selftests/.*/Makefile?
We can see if there are false-negatives.
> + fi
> +done
> +
> +exit $rc
> diff --git a/tests/patch/check_selftest/info.json b/tests/patch/check_selftest/info.json
> new file mode 100644
> index 0000000..615779f
> --- /dev/null
> +++ b/tests/patch/check_selftest/info.json
> @@ -0,0 +1,3 @@
> +{
> + "run": ["check_selftest.sh"]
> +}
^ permalink raw reply
* [PATCH net v1 2/2] net: dsa: lantiq_gswip: Fix typo in gswip_port_fdb_dump() error print
From: Martin Blumenstingl @ 2022-05-17 19:40 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, andrew, vivien.didelot, f.fainelli, olteanv, davem,
edumazet, kuba, pabeni, Martin Blumenstingl, Hauke Mehrtens
In-Reply-To: <20220517194015.1081632-1-martin.blumenstingl@googlemail.com>
gswip_port_fdb_dump() reads the MAC bridge entries. The error message
should say "failed to read mac bridge entry". While here, also add the
index to the error print so humans can get to the cause of the problem
easier.
Fixes: 58c59ef9e930c4 ("net: dsa: lantiq: Add Forwarding Database access")
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/net/dsa/lantiq_gswip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 0c313db23451..8af4def38a98 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1426,8 +1426,9 @@ static int gswip_port_fdb_dump(struct dsa_switch *ds, int port,
err = gswip_pce_table_entry_read(priv, &mac_bridge);
if (err) {
- dev_err(priv->dev, "failed to write mac bridge: %d\n",
- err);
+ dev_err(priv->dev,
+ "failed to read mac bridge entry %d: %d\n",
+ i, err);
return err;
}
--
2.36.1
^ permalink raw reply related
* [PATCH net v1 1/2] net: dsa: lantiq_gswip: Fix start index in gswip_port_fdb()
From: Martin Blumenstingl @ 2022-05-17 19:40 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, andrew, vivien.didelot, f.fainelli, olteanv, davem,
edumazet, kuba, pabeni, Martin Blumenstingl, Hauke Mehrtens
In-Reply-To: <20220517194015.1081632-1-martin.blumenstingl@googlemail.com>
The first N entries in priv->vlans are reserved for managing ports which
are not part of a bridge. Use priv->hw_info->max_ports to consistently
access per-bridge entries at index 7. Starting at
priv->hw_info->cpu_port (6) is harmless in this case because
priv->vlan[6].bridge is always NULL so the comparison result is always
false (which results in this entry being skipped).
Fixes: 58c59ef9e930c4 ("net: dsa: lantiq: Add Forwarding Database access")
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/net/dsa/lantiq_gswip.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 12c15da55664..0c313db23451 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1360,7 +1360,7 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
struct net_device *bridge = dsa_port_bridge_dev_get(dsa_to_port(ds, port));
struct gswip_priv *priv = ds->priv;
struct gswip_pce_table_entry mac_bridge = {0,};
- unsigned int cpu_port = priv->hw_info->cpu_port;
+ unsigned int max_ports = priv->hw_info->max_ports;
int fid = -1;
int i;
int err;
@@ -1368,7 +1368,7 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
if (!bridge)
return -EINVAL;
- for (i = cpu_port; i < ARRAY_SIZE(priv->vlans); i++) {
+ for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
if (priv->vlans[i].bridge == bridge) {
fid = priv->vlans[i].fid;
break;
--
2.36.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