* [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode
@ 2026-09-29 18:58 Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 1/6] net: add netif_rx_mode_schedule_update() Satish Kharat
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
An ENIC V2 VF cannot program its station address or receive filters
directly. The PF owns those resources and exposes mailbox operations for
the VF to request changes. Add the VF control plane needed to use those
operations for station-address, address-list, and packet-filter
configuration.
The mailbox transport remains asynchronous: receive dispatch continues to
process replies and unsolicited PF notifications. The VF allows only one
request/reply transaction at a time because it stores one expected reply
and uses one completion. This serialization applies only to VF requests;
PF-side request processing is unchanged. Notifications and their
acknowledgments remain deferred and asynchronous.
The first patch adds a netdev helper for scheduling an immediate
receive-mode update with a new retry budget. Callers which need to replay
lost address state explicitly reset the corresponding synchronization state
before scheduling the update. The second patch serializes only VF requests.
The third patch recovers when a lost or malformed reply means that the VF
does not know which changes the PF applied. A hardware send timeout is
handled separately because the descriptor may still belong to the device.
The remaining patches add and validate the configuration operations,
manage station and administrative MAC changes, and install address lists
and packet-filter settings through the sleepable receive-mode path. Replies
are checked against the requested operation. If a station-address
replacement deletes the old address but fails to add the new one, the VF
registers again and rebuilds its mailbox state instead of assuming which
address the PF kept.
This is the VF-side receive-control series. It does not register
.sriov_configure, so the in-tree V2 PF path remains dormant. A later PF
activation series must implement the PF side of the MAC-address and
packet-filter mailbox requests, including the VF policy checks, before
wiring that callback.
AI assistance:
An LLM was used for design review, source review, commit-message
drafting, test automation, and review triage. Sashiko was used as an
additional review tool. All findings and generated changes were manually
reviewed, and the author takes responsibility for the series.
Validation:
- The exact v2 VF module against the deployed async PF passed 32 secondary
unicast addresses, station-address add/delete through the multicast list,
and fixed administrative-MAC enforcement across a close/open.
- Isolated test-only fault and timing controls verified bounded behavior
under persistent registration rejection, no reconnect after a coherent
generic MAC-operation rejection, convergence when an address changed
during receive-mode processing, and synchronous broad-to-exact filter
completion.
- With all injections disabled, the exact v2 VF passed 20 multicast
add/delete operations, three close/open cycles, statistics retrieval, and
bidirectional traffic. Bounded host and guest kernel-health scans found
no new fault report. The installed kernel was not rebuilt or rebooted.
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
Changes in v2:
- Rename the netdev helper to netif_rx_mode_schedule_update() and clarify
that it resets retry state and schedules reconciliation without itself
forcing a complete address-list replay.
- Stop a failed registration-recovery handshake from requeueing its own
reset worker indefinitely.
- Keep coherent generic MAC-operation failures as ordinary errors;
reconnect only after explicit registration loss or an indeterminate reply.
- Enforce a cached fixed administrative MAC while the VF is down and repair
an actual device-address mismatch during registration.
- Preserve compatibility with deployed async PFs at 32 secondary unicast
addresses and exclude the station address from both secondary lists.
- Complete receive-mode transitions in one callback so concurrent list
changes converge and synchronous operations return with filters
installed.
- Link to v1: https://patch.msgid.link/20260921-b4-enic-sriov-v2-vf-receive-control-v1-0-67a0a6e08d43@cisco.com
---
Satish Kharat (6):
net: add netif_rx_mode_schedule_update()
enic: serialize V2 VF mailbox requests
enic: recover V2 VF mailbox when PF state is unknown
enic: validate V2 VF configuration replies
enic: manage V2 VF station and administrative MAC
enic: configure V2 VF receive mode over mailbox
drivers/net/ethernet/cisco/enic/enic.h | 58 +-
drivers/net/ethernet/cisco/enic/enic_admin.c | 29 +-
drivers/net/ethernet/cisco/enic/enic_dev.c | 11 +
drivers/net/ethernet/cisco/enic/enic_dev.h | 1 +
drivers/net/ethernet/cisco/enic/enic_main.c | 1251 +++++++++++++++++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.c | 739 ++++++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.h | 84 ++
drivers/net/ethernet/cisco/enic/enic_rq.c | 11 +-
include/linux/netdevice.h | 1 +
net/core/dev_addr_lists.c | 21 +
10 files changed, 2126 insertions(+), 80 deletions(-)
---
base-commit: 8830e65ed46de41f849eefb8ba227d4852c460f6
change-id: 20260916-b4-enic-sriov-v2-vf-receive-control-b5c40bdae5b9
Best regards,
--
Satish Kharat <satishkh@cisco.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 1/6] net: add netif_rx_mode_schedule_update()
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
@ 2026-09-29 18:58 ` Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 2/6] enic: serialize V2 VF mailbox requests Satish Kharat
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
Add a receive-mode scheduling helper for callers which start a new update
after a state transition. Cancel a pending retry and reset its backoff
before queueing the immediate update so the preceding operation's retry
state cannot shorten the new operation's bounded retry sequence.
The helper preserves address-list synchronization state. Callers which need
to replay configuration lost by the device must unsynchronize the affected
address lists before scheduling the update.
Assisted-by: LLM
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
include/linux/netdevice.h | 1 +
net/core/dev_addr_lists.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5d16737167ee..c1807a8f733b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5197,6 +5197,7 @@ static inline void __dev_mc_unsync(struct net_device *dev,
/* Functions used for secondary unicast and multicast support */
void dev_set_rx_mode(struct net_device *dev);
+void netif_rx_mode_schedule_update(struct net_device *dev);
void netif_rx_mode_schedule_retry(struct net_device *dev);
int netif_set_promiscuity(struct net_device *dev, int inc);
int dev_set_promiscuity(struct net_device *dev, int inc);
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 08528ca0a8b3..17770a38b5ab 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -1337,6 +1337,27 @@ static void netif_rx_mode_queue(struct net_device *dev)
__netdev_work_core_sched(dev, NETDEV_WORK_RX_MODE);
}
+/**
+ * netif_rx_mode_schedule_update() - schedule a new receive-mode update
+ * @dev: network device
+ *
+ * Cancel any pending retry and reset its backoff before scheduling an
+ * immediate receive-mode update. This does not reset address-list
+ * synchronization state. Callers which need to replay lost device state must
+ * unsynchronize the affected address lists before scheduling the update.
+ *
+ * Context: sleepable. The caller must hold the device operations lock, or
+ * RTNL for a device which still uses RTNL-compatible operations.
+ */
+void netif_rx_mode_schedule_update(struct net_device *dev)
+{
+ might_sleep();
+ netdev_assert_locked_ops_compat(dev);
+ netif_rx_mode_cancel_retry(dev);
+ netif_rx_mode_queue(dev);
+}
+EXPORT_SYMBOL_GPL(netif_rx_mode_schedule_update);
+
static void netif_rx_mode_retry(struct timer_list *t)
{
struct net_device *dev =
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 2/6] enic: serialize V2 VF mailbox requests
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 1/6] net: add netif_rx_mode_schedule_update() Satish Kharat
@ 2026-09-29 18:58 ` Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown Satish Kharat
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
The VF stores one expected reply and uses one completion for mailbox
requests. If two VF control paths issue requests at the same time, the
second request can replace the reply state for the first.
Add a request mutex used only by the VF. Hold it from before a request is
armed until its reply or timeout has been consumed. Recheck VF registration
after taking the mutex and clear pending state on send failure so a later
request cannot inherit it.
PF-side request processing is unchanged. Unsolicited PF notifications and
their acknowledgments remain asynchronous.
Assisted-by: LLM
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 1 +
drivers/net/ethernet/cisco/enic/enic_mbox.c | 49 +++++++++++++++++++++++++++--
2 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 7a509a056990..3945fe28f199 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -335,6 +335,7 @@ struct enic {
* the requester.
*/
struct completion mbox_comp;
+ struct mutex vf_mbox_request_lock; /* serializes VF request lifetimes */
spinlock_t mbox_state_lock; /* protects expected reply state */
u64 mbox_expected_msg_num;
u8 mbox_expected_reply;
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index 5c93ca49552a..b8a18d9682b2 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -218,6 +218,32 @@ static int enic_mbox_wait_reply(struct enic *enic, unsigned long timeout_ms)
return err;
}
+static void enic_mbox_vf_request_start(struct enic *enic)
+{
+ mutex_lock(&enic->vf_mbox_request_lock);
+ reinit_completion(&enic->mbox_comp);
+ spin_lock_bh(&enic->mbox_state_lock);
+ enic->mbox_expected_msg_num = 0;
+ enic->mbox_expected_reply = 0;
+ spin_unlock_bh(&enic->mbox_state_lock);
+}
+
+static void enic_mbox_vf_request_abort(struct enic *enic)
+{
+ lockdep_assert_held(&enic->vf_mbox_request_lock);
+ spin_lock_bh(&enic->mbox_state_lock);
+ enic->mbox_expected_reply = 0;
+ enic->mbox_expected_msg_num = 0;
+ spin_unlock_bh(&enic->mbox_state_lock);
+ mutex_unlock(&enic->vf_mbox_request_lock);
+}
+
+static void enic_mbox_vf_request_finish(struct enic *enic)
+{
+ lockdep_assert_held(&enic->vf_mbox_request_lock);
+ mutex_unlock(&enic->vf_mbox_request_lock);
+}
+
int enic_mbox_send_link_state(struct enic *enic, u16 vf_id, u32 link_state)
{
struct enic_mbox_pf_link_state_notif_msg notif = {};
@@ -623,6 +649,7 @@ int enic_mbox_vf_capability_check(struct enic *enic)
u32 version;
int err;
+ enic_mbox_vf_request_start(enic);
WRITE_ONCE(enic->pf_cap_version, 0);
req.version = cpu_to_le32(ENIC_MBOX_CAP_VERSION_1);
@@ -630,11 +657,14 @@ int enic_mbox_vf_capability_check(struct enic *enic)
ENIC_MBOX_VF_CAPABILITY_REQUEST,
ENIC_MBOX_VF_CAPABILITY_REPLY,
&req, sizeof(req));
- if (err)
+ if (err) {
+ enic_mbox_vf_request_abort(enic);
return err;
+ }
err = enic_mbox_wait_reply(enic, 3000);
version = READ_ONCE(enic->pf_cap_version);
+ enic_mbox_vf_request_finish(enic);
if (err) {
netdev_warn(enic->netdev,
"MBOX: no capability reply from PF\n");
@@ -656,15 +686,19 @@ int enic_mbox_vf_register(struct enic *enic)
bool registered;
int err;
+ enic_mbox_vf_request_start(enic);
WRITE_ONCE(enic->vf_registered, false);
err = enic_mbox_vf_send_request(enic, ENIC_MBOX_VF_REGISTER_REQUEST,
ENIC_MBOX_VF_REGISTER_REPLY, NULL, 0);
- if (err)
+ if (err) {
+ enic_mbox_vf_request_abort(enic);
return err;
+ }
err = enic_mbox_wait_reply(enic, 3000);
registered = READ_ONCE(enic->vf_registered);
+ enic_mbox_vf_request_finish(enic);
if (err) {
netdev_warn(enic->netdev,
"MBOX: VF registration with PF timed out\n");
@@ -684,16 +718,24 @@ int enic_mbox_vf_unregister(struct enic *enic)
if (!READ_ONCE(enic->vf_registered))
return 0;
+ enic_mbox_vf_request_start(enic);
+ if (!READ_ONCE(enic->vf_registered)) {
+ enic_mbox_vf_request_finish(enic);
+ return 0;
+ }
err = enic_mbox_vf_send_request(enic,
ENIC_MBOX_VF_UNREGISTER_REQUEST,
ENIC_MBOX_VF_UNREGISTER_REPLY,
NULL, 0);
- if (err)
+ if (err) {
+ enic_mbox_vf_request_abort(enic);
return err;
+ }
err = enic_mbox_wait_reply(enic, 3000);
registered = READ_ONCE(enic->vf_registered);
+ enic_mbox_vf_request_finish(enic);
if (err)
return err;
if (registered)
@@ -711,6 +753,7 @@ void enic_mbox_init(struct enic *enic)
*/
if (!reinit) {
mutex_init(&enic->mbox_lock);
+ mutex_init(&enic->vf_mbox_request_lock);
init_completion(&enic->mbox_comp);
spin_lock_init(&enic->mbox_state_lock);
enic->mbox_msg_num = 0;
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 1/6] net: add netif_rx_mode_schedule_update() Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 2/6] enic: serialize V2 VF mailbox requests Satish Kharat
@ 2026-09-29 18:58 ` Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
2026-09-29 18:58 ` [PATCH net-next v2 4/6] enic: validate V2 VF configuration replies Satish Kharat
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
A mailbox send can complete and still lose its reply. In that case the VF
cannot know whether the PF changed its address or receive-filter state. A
send-completion timeout is different because the admin-WQ descriptor may
still belong to the device.
Reconnect the V2 VF mailbox after the first case. After a hardware send
timeout, stop using the channel and leave the timed-out DMA mapping for
admin-channel teardown to reclaim safely.
Stop V2 VF receive traffic when the VF can no longer trust that its state
matches the PF. Rebuild and register the admin channel again at a safe
open/reset boundary, then restore receive traffic after the station address
and filters have been replayed. Defer notification acknowledgments so
receive dispatch cannot block behind a VF request.
Do not let a failed recovery handshake continuously requeue the worker
which is attempting the reset. Keep the VF quarantined after that failed
attempt until an administrative close/open or another external recovery
event.
Reinitialize non-dynamic vNICs after a successful soft reset before
programming queue resources. This restores the initialization sequence used
during probe and ensures that reset recovery rebuilds the vNIC before
reopening the datapath.
Also use this recovery after malformed or dropped admin receive traffic,
and track whether the V2 datapath is open so a failed internal reset cannot
stop it twice.
Assisted-by: LLM
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 28 +++-
drivers/net/ethernet/cisco/enic/enic_admin.c | 27 +++-
drivers/net/ethernet/cisco/enic/enic_main.c | 219 +++++++++++++++++++++++----
drivers/net/ethernet/cisco/enic/enic_mbox.c | 172 +++++++++++++++++++--
drivers/net/ethernet/cisco/enic/enic_mbox.h | 2 +
drivers/net/ethernet/cisco/enic/enic_rq.c | 11 +-
6 files changed, 409 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 3945fe28f199..4285b63083cc 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -303,8 +303,26 @@ struct enic {
* left the resources freed.
*/
bool admin_chan_up;
- /* set on send timeout; cleared on channel re-open */
+ /* Blocks sends while the channel is closed or awaiting recovery. */
bool mbox_send_disabled;
+ /* A send timeout leaves a descriptor hardware-owned. Do not reopen the
+ * channel during this device lifetime until reset/DMA fencing is proven.
+ */
+ bool mbox_tx_poisoned;
+ /* After a lost or inconsistent reply, the VF cannot know whether the PF
+ * applied the request. Reconnect during the next open or reset.
+ */
+ bool vf_mbox_reconnect_required;
+ /* Suppress self-requeue while a reset worker is already attempting the
+ * reconnect. A failed handshake remains quarantined until a later external
+ * recovery event or administrative close/open.
+ */
+ bool vf_mbox_recovery_active;
+ u32 vf_mbox_fault_generation;
+ /* One slow-path-owned predicate keeps the RX hot path fail-closed while
+ * VF registration is lost or receive state may not match the PF.
+ */
+ bool vf_rx_quarantined;
struct vnic_wq admin_wq;
struct vnic_rq admin_rq;
struct vnic_cq admin_cq[2];
@@ -325,6 +343,10 @@ struct enic {
spinlock_t vf_link_state_lock;
enum enic_vf_link_state vf_link_state;
bool vf_link_running;
+ /* Tracks a completely opened V2 VF datapath. An internal reset can stop
+ * it while netif_running() remains true, then fail before reopen.
+ */
+ bool vf_datapath_open;
/* MBOX protocol state — mbox_lock serializes admin WQ sends */
struct mutex mbox_lock;
@@ -337,6 +359,10 @@ struct enic {
struct completion mbox_comp;
struct mutex vf_mbox_request_lock; /* serializes VF request lifetimes */
spinlock_t mbox_state_lock; /* protects expected reply state */
+ spinlock_t vf_ack_lock; /* protects vf_ack_list */
+ struct list_head vf_ack_list;
+ struct work_struct vf_ack_work;
+ unsigned int vf_ack_count;
u64 mbox_expected_msg_num;
u8 mbox_expected_reply;
bool mbox_initialized;
diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c
index 61c82b48044d..30c0a5c89a2d 100644
--- a/drivers/net/ethernet/cisco/enic/enic_admin.c
+++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
@@ -132,14 +132,22 @@ unsigned int enic_admin_wq_cq_service(struct enic *enic)
*/
#define ENIC_ADMIN_MSG_MAX 256
+static void enic_admin_rx_lost(struct enic *enic)
+{
+ if (enic_is_sriov_vf_v2(enic))
+ enic_mbox_vf_require_reconnect(enic);
+}
+
static void enic_admin_msg_enqueue(struct enic *enic, void *buf,
unsigned int len)
{
struct enic_admin_msg *msg;
msg = kmalloc_flex(*msg, data, len);
- if (!msg)
+ if (!msg) {
+ enic_admin_rx_lost(enic);
return;
+ }
msg->len = len;
memcpy(msg->data, buf, len);
@@ -152,6 +160,7 @@ static void enic_admin_msg_enqueue(struct enic *enic, void *buf,
netdev_warn(enic->netdev,
"admin msg backlog full (%u); dropping\n",
ENIC_ADMIN_MSG_MAX);
+ enic_admin_rx_lost(enic);
return;
}
list_add_tail(&msg->list, &enic->admin_msg_list);
@@ -194,8 +203,10 @@ unsigned int enic_admin_rq_cq_service(struct enic *enic)
rq_desc = desc;
bwf = le16_to_cpu(rq_desc->bytes_written_flags);
bytes_written = bwf & CQ_ENET_RQ_DESC_BYTES_WRITTEN_MASK;
- if (bytes_written > buf->len)
+ if (bytes_written > buf->len) {
+ enic_admin_rx_lost(enic);
goto next_desc;
+ }
dma_sync_single_for_cpu(&enic->pdev->dev,
buf->dma_addr, buf->len,
@@ -210,11 +221,13 @@ unsigned int enic_admin_rq_cq_service(struct enic *enic)
if (bwf & CQ_ENET_RQ_DESC_FLAGS_TRUNCATED) {
netdev_warn_once(enic->netdev,
"admin RQ: truncated message dropped\n");
+ enic_admin_rx_lost(enic);
goto next_desc;
}
if (!(rq_desc->flags & CQ_ENET_RQ_DESC_FLAGS_FCS_OK)) {
netdev_warn_once(enic->netdev,
"admin RQ: bad FCS, dropping message\n");
+ enic_admin_rx_lost(enic);
goto next_desc;
}
@@ -534,6 +547,11 @@ int enic_admin_channel_open(struct enic *enic)
if (!enic->has_admin_channel)
return -ENODEV;
+ if (READ_ONCE(enic->mbox_tx_poisoned)) {
+ netdev_err(enic->netdev,
+ "Refusing to reopen admin channel after send timeout\n");
+ return -EIO;
+ }
/* Keep MBOX sends disabled for the entire open sequence. It is
* cleared only after every resource is allocated and enabled below,
@@ -641,6 +659,11 @@ void enic_admin_channel_close(struct enic *enic)
enic_admin_teardown_intr(enic);
cancel_work_sync(&enic->link_notify_work);
cancel_work_sync(&enic->admin_msg_work);
+ /* admin_msg_work is the sole VF ACK producer. Drain it before the ACK
+ * worker so an enqueue cannot race the final cancel and queue purge.
+ */
+ if (enic_is_sriov_vf_v2(enic))
+ enic_mbox_vf_ack_cancel(enic);
enic_admin_msg_drain(enic);
enic_admin_qp_type_set(enic, QP_DISABLE);
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 9086e6dd558a..28738bd14fa7 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -71,6 +71,8 @@
#define PCI_DEVICE_ID_CISCO_VIC_ENET_VF_V2 0x02b7 /* enet SRIOV V2 VF */
#define PCI_DEVICE_ID_CISCO_VIC_ENET_VF_USNIC 0x00cf /* enet USNIC VF */
+static int __enic_stop(struct net_device *netdev, bool remove_vf_station);
+
/* Supported devices */
static const struct pci_device_id enic_id_table[] = {
{ PCI_VDEVICE(CISCO, PCI_DEVICE_ID_CISCO_VIC_ENET) },
@@ -1718,6 +1720,8 @@ static void enic_notify_timer_start(struct enic *enic)
}
}
+static int enic_admin_chan_reopen(struct enic *enic);
+
/* rtnl lock is held, process context */
static int enic_open(struct net_device *netdev)
{
@@ -1736,6 +1740,30 @@ static int enic_open(struct net_device *netdev)
.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
};
+ /* A reply timeout invalidates the current request generation. Rebuild
+ * and re-register the channel before allocating datapath resources so a
+ * later userspace down/up can recover a failed open or reset handshake.
+ * A send timeout is intentionally not recoverable here because its WQ
+ * descriptor may still be hardware-owned.
+ */
+ if (enic_is_sriov_vf_v2(enic) &&
+ READ_ONCE(enic->mbox_tx_poisoned))
+ return -EIO;
+ if (enic_is_sriov_vf_v2(enic) &&
+ (!enic->admin_chan_up || !READ_ONCE(enic->vf_registered) ||
+ READ_ONCE(enic->vf_mbox_reconnect_required))) {
+ /* Re-registration makes the PF discard the old VF-requested
+ * filters. Clear the netdev-core synchronization state so the
+ * receive-mode callback replays the current address lists.
+ */
+ enic_reset_addr_lists(enic);
+ if (enic->admin_chan_up)
+ enic_admin_channel_close(enic);
+ err = enic_admin_chan_reopen(enic);
+ if (err)
+ return err;
+ }
+
err = enic_request_intr(enic);
if (err) {
netdev_err(netdev, "Unable to request irq.\n");
@@ -1794,17 +1822,41 @@ static int enic_open(struct net_device *netdev)
netdev_err(netdev, "Failed to enable device: %d\n", err);
goto err_out_dev_enable;
}
+ if (enic_is_sriov_vf_v2(enic)) {
+ /* Commit the replay only if no mailbox fault arrived while station
+ * and receive policy were being programmed. Keep the state lock
+ * through the carrier transition so a later fault necessarily wins
+ * and turns carrier back off.
+ */
+ spin_lock_bh(&enic->mbox_state_lock);
+ if (!READ_ONCE(enic->vf_registered) ||
+ READ_ONCE(enic->mbox_send_disabled) ||
+ READ_ONCE(enic->mbox_tx_poisoned) ||
+ READ_ONCE(enic->vf_mbox_reconnect_required)) {
+ err = -EIO;
+ } else {
+ WRITE_ONCE(enic->vf_rx_quarantined, false);
+ enic_mbox_vf_link_state_set_running(enic, true);
+ }
+ spin_unlock_bh(&enic->mbox_state_lock);
+ if (err) {
+ netdev_err(netdev,
+ "MBOX state changed during VF datapath open\n");
+ goto err_out_dev_disable;
+ }
+ }
for (i = 0; i < enic->intr_count; i++)
vnic_intr_unmask(&enic->intr[i]);
-
enic_notify_timer_start(enic);
enic_rfs_timer_start(enic);
if (enic_is_sriov_vf_v2(enic))
- enic_mbox_vf_link_state_set_running(enic, true);
+ enic->vf_datapath_open = true;
return 0;
+err_out_dev_disable:
+ enic_dev_disable(enic);
err_out_dev_enable:
for (i = 0; i < enic->rq_count; i++)
napi_disable(&enic->napi[i]);
@@ -1834,12 +1886,20 @@ static int enic_open(struct net_device *netdev)
}
/* rtnl lock is held, process context */
-static int enic_stop(struct net_device *netdev)
+static int __enic_stop(struct net_device *netdev, bool remove_vf_station)
{
struct enic *enic = netdev_priv(netdev);
unsigned int i;
int err;
+ /* Internal reset leaves netif_running() set while the datapath is down.
+ * If re-registration or reopen then fails, a later administrative close
+ * must not disable NAPI a second time.
+ */
+ if (enic_is_sriov_vf_v2(enic) && !enic->vf_datapath_open)
+ return 0;
+ (void)remove_vf_station;
+
for (i = 0; i < enic->intr_count; i++) {
vnic_intr_mask(&enic->intr[i]);
(void)vnic_intr_masked(&enic->intr[i]); /* flush write */
@@ -1893,10 +1953,17 @@ static int enic_stop(struct net_device *netdev)
vnic_cq_clean(&enic->cq[i]);
for (i = 0; i < enic->intr_count; i++)
vnic_intr_clean(&enic->intr[i]);
+ if (enic_is_sriov_vf_v2(enic))
+ enic->vf_datapath_open = false;
return 0;
}
+static int enic_stop(struct net_device *netdev)
+{
+ return __enic_stop(netdev, true);
+}
+
static int _enic_change_mtu(struct net_device *netdev, int new_mtu)
{
bool running = netif_running(netdev);
@@ -2196,14 +2263,15 @@ static bool enic_has_admin_chan(struct enic *enic)
(enic_sriov_enabled(enic) && enic->vf_type == ENIC_VF_TYPE_V2);
}
-/* Re-establish the admin/MBOX channel after a reset has re-created the data
- * path. Mirrors the relevant part of the probe / SR-IOV-enable sequence:
+/* Re-establish the admin/MBOX channel after a reset has re-created the vNIC
+ * resources. Mirrors the relevant part of the probe / SR-IOV-enable sequence:
* reinitialise MBOX and reopen the channel, then for a VF re-run the PF
* handshake (the reset wiped the VF's admin QP, so the VF must register
* again), or for a PF re-push the current link state to registered VFs.
*/
-static void enic_admin_chan_reopen(struct enic *enic)
+static int enic_admin_chan_reopen(struct enic *enic)
{
+ u32 recovery_generation = 0;
int err;
/* Install the MBOX receive handler and clear pending reply state before
@@ -2222,12 +2290,17 @@ static void enic_admin_chan_reopen(struct enic *enic)
*/
if (enic_is_sriov_vf_v2(enic))
WRITE_ONCE(enic->vf_registered, false);
+ if (enic_is_sriov_vf_v2(enic)) {
+ spin_lock_bh(&enic->mbox_state_lock);
+ recovery_generation = enic->vf_mbox_fault_generation;
+ spin_unlock_bh(&enic->mbox_state_lock);
+ }
err = enic_admin_channel_open(enic);
if (err) {
netdev_err(enic->netdev,
"admin channel reopen after reset failed: %d\n", err);
- return;
+ return err;
}
if (enic_is_sriov_vf_v2(enic)) {
@@ -2237,7 +2310,7 @@ static void enic_admin_chan_reopen(struct enic *enic)
"MBOX capability check after reset failed: %d\n",
err);
enic_admin_channel_close(enic);
- return;
+ return err;
}
err = enic_mbox_vf_register(enic);
if (err) {
@@ -2245,6 +2318,26 @@ static void enic_admin_chan_reopen(struct enic *enic)
"MBOX VF re-registration after reset failed: %d\n",
err);
enic_admin_channel_close(enic);
+ return err;
+ }
+ enic_reset_addr_lists(enic);
+ /* Capability negotiation and registration establish a new protocol
+ * generation. RX remains quarantined until enic_open() replays the
+ * station and receive policy.
+ */
+ spin_lock_bh(&enic->mbox_state_lock);
+ if (enic->vf_mbox_fault_generation != recovery_generation ||
+ READ_ONCE(enic->mbox_tx_poisoned)) {
+ err = -EAGAIN;
+ } else {
+ WRITE_ONCE(enic->vf_mbox_reconnect_required, false);
+ }
+ spin_unlock_bh(&enic->mbox_state_lock);
+ if (err) {
+ netdev_warn(enic->netdev,
+ "MBOX state changed during VF re-registration\n");
+ enic_admin_channel_close(enic);
+ return err;
}
} else {
/* The link came back up during enic_open() above while MBOX
@@ -2253,79 +2346,128 @@ static void enic_admin_chan_reopen(struct enic *enic)
*/
schedule_work(&enic->link_notify_work);
}
+
+ return 0;
}
static void enic_reset(struct work_struct *work)
{
struct enic *enic = container_of(work, struct enic, reset);
+ bool vf_recovery = enic_is_sriov_vf_v2(enic);
+ int err;
if (!netif_running(enic->netdev))
return;
+ if (vf_recovery)
+ WRITE_ONCE(enic->vf_mbox_recovery_active, true);
rtnl_lock();
+ /* V2 protocol recovery can be queued immediately before ndo_stop()
+ * acquires RTNL. Recheck under RTNL so that new recovery path cannot
+ * reopen a device userspace just closed. Preserve the existing reset
+ * behavior for every other ENIC device.
+ */
+ if (enic_is_sriov_vf_v2(enic) && !netif_running(enic->netdev))
+ goto unlock;
/* Stop any activity from infiniband */
enic_set_api_busy(enic, true);
- /* Fully tear down the V2 admin/MBOX channel before the soft reset.
- * The reset wipes all hardware queues including the admin WQ/RQ;
- * closing first tells firmware to stop the admin QP (so it no longer
- * DMAs from the about-to-be-reset rings) and frees the admin resources
- * so they are cleanly re-allocated afterwards.
+ /* Stop the datapath and existing admin/MBOX channel before the soft
+ * reset. Do not send DEL_MAC from this path: a timeout would poison the
+ * channel while reset and fresh registration already discard the old
+ * VF-requested protocol state before the station address is replayed.
+ * Reopen allocates fresh admin resources after reset recreates the vNIC.
*/
+ __enic_stop(enic->netdev, false);
if (enic_has_admin_chan(enic))
enic_admin_channel_close(enic);
- enic_stop(enic->netdev);
if (enic_is_sriov_vf_v2(enic))
enic_mbox_vf_link_state_reset(enic);
+ err = enic_dev_soft_reset(enic);
+ if (err)
+ goto reset_out;
+
+ if (!enic_is_dynamic(enic)) {
+ err = vnic_dev_init(enic->vdev, 0);
+ if (err) {
+ netdev_err(enic->netdev,
+ "vNIC init after soft reset failed: %d\n",
+ err);
+ goto reset_out;
+ }
+ }
- enic_dev_soft_reset(enic);
enic_reset_addr_lists(enic);
enic_init_vnic_resources(enic);
enic_set_rss_nic_cfg(enic);
enic_dev_set_ig_vlan_rewrite_mode(enic);
enic_ext_cq(enic);
- enic_open(enic->netdev);
+ /* A V2 VF needs PF registration before enic_open() can install its
+ * station address. A V2 PF reopens afterwards and replays carrier.
+ */
+ if (enic_is_sriov_vf_v2(enic)) {
+ err = enic_admin_chan_reopen(enic);
+ if (err)
+ goto reset_out;
+ }
+
+ err = enic_open(enic->netdev);
+ if (err)
+ netdev_err(enic->netdev,
+ "Failed to reopen datapath after reset: %d\n", err);
- /* Re-establish the admin/MBOX channel after the data path is back up.
- * It was fully torn down by enic_admin_channel_close() above;
- * enic_admin_chan_reopen() reopens it and, for a PF re-pushes link
- * state, or for a VF re-runs the probe-time PF handshake.
+ /* A PF reopens its admin channel after the datapath and re-pushes link
+ * state. The VF handshake, which open depends on, completed above.
*/
- if (enic_has_admin_chan(enic))
+ if (enic_has_admin_chan(enic) && !enic_is_sriov_vf_v2(enic))
enic_admin_chan_reopen(enic);
+reset_out:
/* Allow infiniband to fiddle with the device again */
enic_set_api_busy(enic, false);
call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev);
+unlock:
+ if (vf_recovery)
+ WRITE_ONCE(enic->vf_mbox_recovery_active, false);
rtnl_unlock();
}
static void enic_tx_hang_reset(struct work_struct *work)
{
struct enic *enic = container_of(work, struct enic, tx_hang_reset);
+ bool vf_recovery = enic_is_sriov_vf_v2(enic);
+ int err;
+
+ if (vf_recovery)
+ WRITE_ONCE(enic->vf_mbox_recovery_active, true);
rtnl_lock();
+ /* The V2 changes below add admin-channel recovery to this worker. Do not
+ * let that new path reopen a VF after userspace completed ndo_stop();
+ * leave the existing behavior for other ENIC devices unchanged.
+ */
+ if (enic_is_sriov_vf_v2(enic) && !netif_running(enic->netdev))
+ goto unlock;
/* Stop any activity from infiniband */
enic_set_api_busy(enic, true);
- /* Fully tear down the V2 admin/MBOX channel before the hang reset, for
- * the same reason as the soft reset path: stop the admin QP and free
- * the admin resources before the hardware queues are wiped.
+ /* Preserve the firmware hang-notification contract by reporting the hung
+ * queue before stopping and cleaning it. As in the soft-reset path, skip
+ * DEL_MAC because reset and fresh registration are the cleanup boundary.
*/
+ enic_dev_hang_notify(enic);
+ __enic_stop(enic->netdev, false);
if (enic_has_admin_chan(enic))
enic_admin_channel_close(enic);
- enic_dev_hang_notify(enic);
- enic_stop(enic->netdev);
if (enic_is_sriov_vf_v2(enic))
enic_mbox_vf_link_state_reset(enic);
-
enic_dev_hang_reset(enic);
enic_reset_addr_lists(enic);
enic_init_vnic_resources(enic);
@@ -2333,21 +2475,32 @@ static void enic_tx_hang_reset(struct work_struct *work)
enic_dev_set_ig_vlan_rewrite_mode(enic);
enic_ext_cq(enic);
- enic_open(enic->netdev);
+ if (enic_is_sriov_vf_v2(enic)) {
+ err = enic_admin_chan_reopen(enic);
+ if (err)
+ goto hang_reset_out;
+ }
+
+ err = enic_open(enic->netdev);
+ if (err)
+ netdev_err(enic->netdev,
+ "Failed to reopen datapath after hang reset: %d\n", err);
- /* Re-establish the admin/MBOX channel after the data path is back up.
- * It was fully torn down by enic_admin_channel_close() above;
- * enic_admin_chan_reopen() reopens it and, for a PF re-pushes link
- * state, or for a VF re-runs the probe-time PF handshake.
+ /* A PF reopens its admin channel after the datapath and re-pushes link
+ * state. The VF handshake, which open depends on, completed above.
*/
- if (enic_has_admin_chan(enic))
+ if (enic_has_admin_chan(enic) && !enic_is_sriov_vf_v2(enic))
enic_admin_chan_reopen(enic);
+hang_reset_out:
/* Allow infiniband to fiddle with the device again */
enic_set_api_busy(enic, false);
call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev);
+unlock:
+ if (vf_recovery)
+ WRITE_ONCE(enic->vf_mbox_recovery_active, false);
rtnl_unlock();
}
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index b8a18d9682b2..30a5f6fda676 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -149,7 +149,20 @@ static int enic_mbox_send_msg_id(struct enic *enic, u8 msg_type,
* or free the buffer: the device may still DMA from dma_addr.
* Mark the channel unusable so no further sends are attempted.
*/
+ spin_lock_bh(&enic->mbox_state_lock);
WRITE_ONCE(enic->mbox_send_disabled, true);
+ WRITE_ONCE(enic->mbox_tx_poisoned, true);
+ if (enic_is_sriov_vf_v2(enic)) {
+ /* The request may have changed PF receive policy even though
+ * local descriptor ownership is still uncertain. Fail the VF
+ * closed and do not turn this into an ordinary protocol
+ * reconnect; the admin-channel lifecycle owns final reclamation.
+ */
+ WRITE_ONCE(enic->vf_rx_quarantined, true);
+ }
+ spin_unlock_bh(&enic->mbox_state_lock);
+ if (enic_is_sriov_vf_v2(enic))
+ enic_mbox_vf_link_state_set_running(enic, false);
}
netdev_dbg(enic->netdev,
@@ -184,6 +197,105 @@ static int enic_mbox_send_reply(struct enic *enic, u8 msg_type,
payload_len, msg_num, true, 0);
}
+struct enic_mbox_vf_ack {
+ struct list_head list;
+ u64 msg_num;
+ u16 ret_major;
+ u8 msg_type;
+};
+
+static void enic_mbox_vf_ack_work(struct work_struct *work)
+{
+ struct enic *enic = container_of(work, struct enic, vf_ack_work);
+ struct enic_mbox_vf_ack *pending;
+
+ for (;;) {
+ struct enic_mbox_generic_reply ack = {};
+ u8 msg_type;
+ int err;
+
+ spin_lock_bh(&enic->vf_ack_lock);
+ if (list_empty(&enic->vf_ack_list)) {
+ spin_unlock_bh(&enic->vf_ack_lock);
+ break;
+ }
+ pending = list_first_entry(&enic->vf_ack_list,
+ struct enic_mbox_vf_ack, list);
+ list_del(&pending->list);
+ enic->vf_ack_count--;
+ spin_unlock_bh(&enic->vf_ack_lock);
+
+ if (READ_ONCE(enic->mbox_send_disabled)) {
+ kfree(pending);
+ continue;
+ }
+
+ msg_type = pending->msg_type;
+ ack.ret_major = cpu_to_le16(pending->ret_major);
+ err = enic_mbox_send_reply(enic, msg_type, ENIC_MBOX_DST_PF,
+ &ack, sizeof(ack), pending->msg_num);
+ kfree(pending);
+ if (err && net_ratelimit())
+ netdev_warn(enic->netdev,
+ "MBOX: failed to send ACK type %u: %d\n",
+ msg_type, err);
+ }
+}
+
+static void enic_mbox_vf_queue_ack(struct enic *enic, u8 msg_type,
+ u64 msg_num, u16 ret_major)
+{
+ struct enic_mbox_vf_ack *pending;
+
+ if (READ_ONCE(enic->mbox_send_disabled))
+ return;
+ pending = kmalloc_obj(*pending, GFP_ATOMIC);
+ if (!pending) {
+ if (net_ratelimit())
+ netdev_warn(enic->netdev,
+ "MBOX: dropping ACK type %u: no memory\n",
+ msg_type);
+ return;
+ }
+ pending->msg_num = msg_num;
+ pending->ret_major = ret_major;
+ pending->msg_type = msg_type;
+
+ spin_lock_bh(&enic->vf_ack_lock);
+ if (READ_ONCE(enic->mbox_send_disabled) ||
+ enic->vf_ack_count >= ENIC_ADMIN_DESC_COUNT) {
+ spin_unlock_bh(&enic->vf_ack_lock);
+ kfree(pending);
+ if (net_ratelimit())
+ netdev_warn(enic->netdev,
+ "MBOX: dropping ACK type %u: queue unavailable\n",
+ msg_type);
+ return;
+ }
+ list_add_tail(&pending->list, &enic->vf_ack_list);
+ enic->vf_ack_count++;
+ spin_unlock_bh(&enic->vf_ack_lock);
+ schedule_work(&enic->vf_ack_work);
+}
+
+void enic_mbox_vf_ack_cancel(struct enic *enic)
+{
+ struct enic_mbox_vf_ack *pending, *tmp;
+ LIST_HEAD(discard);
+
+ if (!enic->mbox_initialized)
+ return;
+ cancel_work_sync(&enic->vf_ack_work);
+ spin_lock_bh(&enic->vf_ack_lock);
+ list_splice_init(&enic->vf_ack_list, &discard);
+ enic->vf_ack_count = 0;
+ spin_unlock_bh(&enic->vf_ack_lock);
+ list_for_each_entry_safe(pending, tmp, &discard, list) {
+ list_del(&pending->list);
+ kfree(pending);
+ }
+}
+
static int enic_mbox_vf_send_request(struct enic *enic, u8 request_type,
u8 expected_reply, void *payload,
u16 payload_len)
@@ -193,6 +305,27 @@ static int enic_mbox_vf_send_request(struct enic *enic, u8 request_type,
expected_reply);
}
+static void enic_mbox_vf_mark_reconnect_locked(struct enic *enic,
+ bool registration_lost)
+{
+ lockdep_assert_held(&enic->mbox_state_lock);
+
+ if (registration_lost)
+ WRITE_ONCE(enic->vf_registered, false);
+ enic->vf_mbox_fault_generation++;
+ WRITE_ONCE(enic->vf_mbox_reconnect_required, true);
+ WRITE_ONCE(enic->mbox_send_disabled, true);
+ WRITE_ONCE(enic->vf_rx_quarantined, true);
+}
+
+static void enic_mbox_vf_kick_recovery(struct enic *enic)
+{
+ enic_mbox_vf_link_state_set_running(enic, false);
+ if (netif_running(enic->netdev) &&
+ !READ_ONCE(enic->vf_mbox_recovery_active))
+ schedule_work(&enic->reset);
+}
+
static int enic_mbox_wait_reply(struct enic *enic, unsigned long timeout_ms)
{
unsigned long left;
@@ -203,9 +336,9 @@ static int enic_mbox_wait_reply(struct enic *enic, unsigned long timeout_ms)
if (left)
return 0;
- /* Invalidate a request that the handler has not already accepted. A
- * delayed reply cannot match a later request because message numbers are
- * monotonic across channel reopen.
+ /* Invalidate a request that the handler has not already accepted. Whether
+ * losing the reply invalidates the current protocol generation is an
+ * operation-specific decision made by the caller.
*/
spin_lock_bh(&enic->mbox_state_lock);
if (enic->mbox_expected_reply) {
@@ -468,9 +601,8 @@ static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload,
u64 msg_num)
{
struct enic_mbox_pf_link_state_notif_msg *notif = payload;
- struct enic_mbox_pf_link_state_ack_msg ack = {};
u32 link_state = le32_to_cpu(notif->link_state);
- int err;
+ u16 ret_major = 0;
spin_lock_bh(&enic->vf_link_state_lock);
switch (link_state) {
@@ -491,16 +623,16 @@ static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload,
default:
netdev_warn(enic->netdev, "MBOX: unknown link state %u\n",
link_state);
- ack.ack.ret_major = cpu_to_le16(ENIC_MBOX_ERR_GENERIC);
+ ret_major = ENIC_MBOX_ERR_GENERIC;
break;
}
spin_unlock_bh(&enic->vf_link_state_lock);
- err = enic_mbox_send_reply(enic, ENIC_MBOX_PF_LINK_STATE_ACK,
- ENIC_MBOX_DST_PF, &ack, sizeof(ack), msg_num);
- if (err && net_ratelimit())
- netdev_warn(enic->netdev,
- "MBOX: failed to send link state ACK: %d\n", err);
+ /* Notification dispatch must not wait behind a synchronous request send:
+ * its matching reply may be queued behind this notification.
+ */
+ enic_mbox_vf_queue_ack(enic, ENIC_MBOX_PF_LINK_STATE_ACK, msg_num,
+ ret_major);
}
void enic_mbox_vf_link_state_reset(struct enic *enic)
@@ -525,6 +657,17 @@ void enic_mbox_vf_link_state_set_running(struct enic *enic, bool running)
spin_unlock_bh(&enic->vf_link_state_lock);
}
+void enic_mbox_vf_require_reconnect(struct enic *enic)
+{
+ /* A fresh REGISTER transaction lets the PF discard any VF-requested
+ * configuration whose final state became uncertain.
+ */
+ spin_lock_bh(&enic->mbox_state_lock);
+ enic_mbox_vf_mark_reconnect_locked(enic, false);
+ spin_unlock_bh(&enic->mbox_state_lock);
+ enic_mbox_vf_kick_recovery(enic);
+}
+
static bool enic_mbox_vf_payload_ok(struct enic *enic, u8 msg_type,
u16 payload_len, size_t min_len)
{
@@ -600,6 +743,8 @@ static void enic_mbox_recv_handler(struct enic *enic, void *buf,
netdev_warn(enic->netdev,
"MBOX: truncated message (len %u < %zu)\n",
len, sizeof(*hdr));
+ if (!enic->vf_state)
+ enic_mbox_vf_require_reconnect(enic);
return;
}
@@ -756,7 +901,12 @@ void enic_mbox_init(struct enic *enic)
mutex_init(&enic->vf_mbox_request_lock);
init_completion(&enic->mbox_comp);
spin_lock_init(&enic->mbox_state_lock);
+ spin_lock_init(&enic->vf_ack_lock);
+ INIT_LIST_HEAD(&enic->vf_ack_list);
+ INIT_WORK(&enic->vf_ack_work, enic_mbox_vf_ack_work);
enic->mbox_msg_num = 0;
+ if (enic_is_sriov_vf_v2(enic))
+ WRITE_ONCE(enic->vf_rx_quarantined, true);
enic->mbox_initialized = true;
} else {
reinit_completion(&enic->mbox_comp);
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.h b/drivers/net/ethernet/cisco/enic/enic_mbox.h
index 60409bad2f28..37bc41a900f4 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.h
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.h
@@ -90,6 +90,8 @@ int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
int enic_mbox_send_link_state(struct enic *enic, u16 vf_id, u32 link_state);
void enic_mbox_vf_link_state_reset(struct enic *enic);
void enic_mbox_vf_link_state_set_running(struct enic *enic, bool running);
+void enic_mbox_vf_ack_cancel(struct enic *enic);
+void enic_mbox_vf_require_reconnect(struct enic *enic);
int enic_mbox_vf_capability_check(struct enic *enic);
int enic_mbox_vf_register(struct enic *enic);
int enic_mbox_vf_unregister(struct enic *enic);
diff --git a/drivers/net/ethernet/cisco/enic/enic_rq.c b/drivers/net/ethernet/cisco/enic/enic_rq.c
index ccbf5c9a21d0..80fe7e819c37 100644
--- a/drivers/net/ethernet/cisco/enic/enic_rq.c
+++ b/drivers/net/ethernet/cisco/enic/enic_rq.c
@@ -330,8 +330,6 @@ static void enic_rq_indicate_buf(struct enic *enic, struct vnic_rq *rq,
u16 bytes_written, vlan_tci, checksum;
u32 rss_hash;
- rqstats->packets++;
-
cq_enet_rq_desc_dec((struct cq_enet_rq_desc *)cq_desc, &ingress_port,
&fcoe, &eop, &sop, &rss_type, &csum_not_calc,
&rss_hash, &bytes_written, &packet_error,
@@ -340,8 +338,15 @@ static void enic_rq_indicate_buf(struct enic *enic, struct vnic_rq *rq,
&tcp_udp_csum_ok, &udp, &tcp, &ipv4_csum_ok, &ipv6,
&ipv4, &ipv4_fragment, &fcs_ok);
- if (enic_rq_pkt_error(rq, packet_error, fcs_ok, bytes_written))
+ if (enic_rq_pkt_error(rq, packet_error, fcs_ok, bytes_written)) {
+ rqstats->packets++;
+ return;
+ }
+ if (unlikely(READ_ONCE(enic->vf_rx_quarantined))) {
+ dev_core_stats_rx_dropped_inc(enic->netdev);
return;
+ }
+ rqstats->packets++;
if (eop && bytes_written > 0) {
/* Good receive
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 4/6] enic: validate V2 VF configuration replies
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
` (2 preceding siblings ...)
2026-09-29 18:58 ` [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown Satish Kharat
@ 2026-09-29 18:58 ` Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
2026-09-29 18:58 ` [PATCH net-next v2 5/6] enic: manage V2 VF station and administrative MAC Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 6/6] enic: configure V2 VF receive mode over mailbox Satish Kharat
5 siblings, 1 reply; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
Add the established V2 mailbox operations for MAC filters,
administrative-MAC notifications, and packet-filter settings. Keep request
buffers alive until the VF request completes so detailed replies can be
checked entry by entry.
Validate message framing, echoed operations, result counts,
operation-specific idempotent results, and applied packet-filter flags
before publishing a reply. If a reply is malformed or contradictory, or
the PF reports that the VF is no longer registered, the VF can no longer
trust that its state matches the PF. Require a new VF registration before
accepting traffic again.
The protocol's ret_minor field counts non-SKIPPED per-entry result codes,
including the idempotent DUPLICATE and NOT_FOUND outcomes. SKIPPED remains
an operation-specific policy result but is not part of that aggregate
count.
Return stable policy errors to callers while retaining retry semantics for
operations that made no state change. A coherent top-level MAC-operation
failure remains an ordinary error. Reserve reconnect for malformed or
indeterminate replies and explicit registration loss.
This patch adds only the VF side of these operations. The in-tree V2 PF
enable path remains dormant because enic_driver does not yet register
.sriov_configure. A future PF activation series must implement PF-side
handling for the MAC-address and packet-filter mailbox requests, including
the VF policy checks, before wiring that callback.
Assisted-by: LLM
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 9 +
drivers/net/ethernet/cisco/enic/enic_mbox.c | 490 +++++++++++++++++++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.h | 82 +++++
3 files changed, 563 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 4285b63083cc..ef8332138c5a 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -239,6 +239,8 @@ enum enic_vf_type {
};
/* Per-instance private data structure */
+struct enic_mac_addr;
+
struct enic {
struct net_device *netdev;
struct pci_dev *pdev;
@@ -365,6 +367,13 @@ struct enic {
unsigned int vf_ack_count;
u64 mbox_expected_msg_num;
u8 mbox_expected_reply;
+ int mbox_reply_status;
+ u16 mbox_reply_filter_flags;
+ /* The request mutex keeps this caller-owned reply array alive until the
+ * matching reply handler has copied all per-address result flags.
+ */
+ struct enic_mac_addr *mbox_reply_mac_addrs;
+ u16 mbox_reply_mac_count;
bool mbox_initialized;
/* PF: per-VF MBOX state, allocated when SRIOV V2 is enabled */
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index 30a5f6fda676..265b58cd39be 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -6,6 +6,7 @@
#include <linux/dma-mapping.h>
#include <linux/delay.h>
#include <linux/completion.h>
+#include <linux/etherdevice.h>
#include "vnic_dev.h"
#include "vnic_wq.h"
@@ -377,6 +378,82 @@ static void enic_mbox_vf_request_finish(struct enic *enic)
mutex_unlock(&enic->vf_mbox_request_lock);
}
+/* Return with mbox_state_lock held when this handler owns the reply. */
+static bool enic_mbox_vf_reply_claim(struct enic *enic, u8 reply_type,
+ u64 msg_num, u8 *expected)
+{
+ spin_lock_bh(&enic->mbox_state_lock);
+ *expected = enic->mbox_expected_reply;
+ if (*expected == reply_type &&
+ enic->mbox_expected_msg_num == msg_num)
+ return true;
+ spin_unlock_bh(&enic->mbox_state_lock);
+
+ return false;
+}
+
+enum enic_mbox_vf_reply_recovery {
+ ENIC_MBOX_VF_REPLY_OK,
+ ENIC_MBOX_VF_REPLY_RECONNECT,
+ ENIC_MBOX_VF_REPLY_REGISTRATION_LOST,
+};
+
+static int
+enic_mbox_vf_classify_reply(bool malformed, u16 ret_major,
+ enum enic_mbox_vf_reply_recovery *recovery)
+{
+ *recovery = ENIC_MBOX_VF_REPLY_OK;
+ if (malformed) {
+ *recovery = ENIC_MBOX_VF_REPLY_RECONNECT;
+ return -EIO;
+ }
+ /* Some deployed peers return a negative errno in this 16-bit field.
+ * Interpret protocol bits only when no unknown bits are present; otherwise
+ * an errno such as -EINVAL could accidentally look like registration loss.
+ */
+ if (!(ret_major & ~ENIC_MBOX_ERR_MASK) &&
+ (ret_major & ENIC_MBOX_ERR_VF_NOT_REGISTERED)) {
+ *recovery = ENIC_MBOX_VF_REPLY_REGISTRATION_LOST;
+ return -ENODEV;
+ }
+ if (!(ret_major & ~ENIC_MBOX_ERR_MASK) &&
+ (ret_major & ENIC_MBOX_ERR_MSG_NOT_SUPPORTED))
+ return -EOPNOTSUPP;
+ if (ret_major)
+ return -EIO;
+
+ return 0;
+}
+
+static void
+enic_mbox_vf_recover_reply_locked(struct enic *enic,
+ enum enic_mbox_vf_reply_recovery recovery)
+{
+ bool registration_lost;
+
+ lockdep_assert_held(&enic->mbox_state_lock);
+
+ if (recovery != ENIC_MBOX_VF_REPLY_OK) {
+ registration_lost =
+ recovery == ENIC_MBOX_VF_REPLY_REGISTRATION_LOST;
+ enic_mbox_vf_mark_reconnect_locked(enic, registration_lost);
+ }
+}
+
+static void enic_mbox_vf_reply_complete(struct enic *enic)
+{
+ lockdep_assert_held(&enic->mbox_state_lock);
+ enic->mbox_expected_reply = 0;
+ enic->mbox_expected_msg_num = 0;
+ /* Publish completion before releasing the state lock. A waiter that
+ * hit the timeout boundary may otherwise see the claimed state, finish the
+ * request, and let a new request reinitialize this completion before the
+ * old handler signals it.
+ */
+ complete(&enic->mbox_comp);
+ spin_unlock_bh(&enic->mbox_state_lock);
+}
+
int enic_mbox_send_link_state(struct enic *enic, u16 vf_id, u32 link_state)
{
struct enic_mbox_pf_link_state_notif_msg notif = {};
@@ -552,23 +629,21 @@ static void enic_mbox_vf_handle_reply(struct enic *enic, u8 reply_type,
void *payload, u64 msg_num)
{
struct enic_mbox_generic_reply *reply = payload;
+ enum enic_mbox_vf_reply_recovery recovery;
u16 ret_major = le16_to_cpu(reply->ret_major);
- u64 expected_msg_num;
- u8 expected_type;
+ u8 expected;
+ int status;
- spin_lock_bh(&enic->mbox_state_lock);
- expected_type = enic->mbox_expected_reply;
- expected_msg_num = enic->mbox_expected_msg_num;
- if (expected_type != reply_type || expected_msg_num != msg_num) {
- spin_unlock_bh(&enic->mbox_state_lock);
+ status = enic_mbox_vf_classify_reply(false, ret_major, &recovery);
+ if (!enic_mbox_vf_reply_claim(enic, reply_type, msg_num, &expected)) {
netdev_warn(enic->netdev,
- "MBOX: stale reply %u/%llu (expected %u/%llu), drop\n",
+ "MBOX: stale reply %u/%llu (expected %u), drop\n",
reply_type, (unsigned long long)msg_num,
- expected_type, (unsigned long long)expected_msg_num);
+ expected);
return;
}
- if (!ret_major) {
+ if (!status) {
switch (reply_type) {
case ENIC_MBOX_VF_CAPABILITY_REPLY: {
struct enic_mbox_vf_capability_reply_msg *cap = payload;
@@ -585,16 +660,166 @@ static void enic_mbox_vf_handle_reply(struct enic *enic, u8 reply_type,
break;
}
}
- enic->mbox_expected_reply = 0;
- enic->mbox_expected_msg_num = 0;
- complete(&enic->mbox_comp);
- spin_unlock_bh(&enic->mbox_state_lock);
+ enic_mbox_vf_recover_reply_locked(enic, recovery);
+ WRITE_ONCE(enic->mbox_reply_status, status);
+ enic_mbox_vf_reply_complete(enic);
if (ret_major)
netdev_warn(enic->netdev,
"MBOX: PF rejected reply type %u: %u/%u\n",
reply_type, ret_major,
le16_to_cpu(reply->ret_minor));
+ if (recovery != ENIC_MBOX_VF_REPLY_OK)
+ enic_mbox_vf_kick_recovery(enic);
+}
+
+static bool enic_mbox_vf_mac_reply_matches(const struct enic_mac_addr *request,
+ const struct enic_mac_addr *reply)
+{
+ u16 request_flags = le16_to_cpu(request->flags);
+ u16 reply_flags = le16_to_cpu(reply->flags);
+ u16 idempotent_result = reply_flags &
+ (ENIC_MAC_ADDR_FLAG_DUPLICATE |
+ ENIC_MAC_ADDR_FLAG_NOT_FOUND);
+ u16 result = reply_flags & ENIC_MAC_ADDR_FLAG_REPLY_MASK;
+ u16 expected_result;
+
+ if (!ether_addr_equal(request->addr, reply->addr) ||
+ (request_flags & ENIC_MAC_ADDR_FLAG_REQUEST_MASK) !=
+ (reply_flags & ENIC_MAC_ADDR_FLAG_REQUEST_MASK))
+ return false;
+ if (hweight16(result) > 1)
+ return false;
+
+ /* DUPLICATE is a successful ADD result and NOT_FOUND is a successful
+ * DELETE result. Neither is valid for the opposite operation, and a
+ * reply cannot report both outcomes for one entry.
+ */
+ expected_result = request_flags & ENIC_MAC_ADDR_FLAG_ADD ?
+ ENIC_MAC_ADDR_FLAG_DUPLICATE :
+ ENIC_MAC_ADDR_FLAG_NOT_FOUND;
+
+ return !idempotent_result || idempotent_result == expected_result;
+}
+
+static void enic_mbox_vf_handle_add_del_mac_reply(struct enic *enic,
+ void *payload, u16 msg_len,
+ u64 msg_num)
+{
+ struct enic_mbox_vf_add_del_mac_reply_msg *reply = payload;
+ enum enic_mbox_vf_reply_recovery recovery;
+ u16 reported_errors = 0;
+ u16 num_addrs = 0;
+ u16 ret_minor = 0;
+ u16 ret_major = 0;
+ u8 expected;
+ unsigned int i;
+ int status;
+
+ if (msg_len < sizeof(*reply)) {
+ status = enic_mbox_vf_classify_reply(true, 0, &recovery);
+ } else {
+ ret_major = le16_to_cpu(reply->reply.ret_major);
+ ret_minor = le16_to_cpu(reply->reply.ret_minor);
+ status = enic_mbox_vf_classify_reply(false, ret_major,
+ &recovery);
+ }
+ if (status)
+ goto claim;
+
+ num_addrs = le16_to_cpu(reply->num_addrs);
+ if (!num_addrs || num_addrs > ENIC_MBOX_MAX_MAC_OPS ||
+ struct_size(reply, mac_addr, num_addrs) > msg_len) {
+ status = enic_mbox_vf_classify_reply(true, 0, &recovery);
+ goto claim;
+ }
+
+claim:
+ if (!enic_mbox_vf_reply_claim(enic, ENIC_MBOX_VF_ADD_DEL_MAC_REPLY,
+ msg_num, &expected))
+ return;
+ if (!status &&
+ (num_addrs != enic->mbox_reply_mac_count ||
+ !enic->mbox_reply_mac_addrs)) {
+ status = enic_mbox_vf_classify_reply(true, 0, &recovery);
+ } else if (!status) {
+ /* A detailed reply corresponds entry-for-entry with the request.
+ * Validate the echoed request fields and operation-specific results
+ * before exposing result flags to the waiting caller.
+ */
+ for (i = 0; i < num_addrs; i++) {
+ struct enic_mac_addr *request =
+ &enic->mbox_reply_mac_addrs[i];
+ u16 flags = le16_to_cpu(reply->mac_addr[i].flags);
+
+ if (!enic_mbox_vf_mac_reply_matches(request,
+ &reply->mac_addr[i])) {
+ status = enic_mbox_vf_classify_reply(true, 0,
+ &recovery);
+ break;
+ }
+ if (flags & ENIC_MAC_ADDR_FLAG_INDETERMINATE_MASK) {
+ status = -EIO;
+ recovery = ENIC_MBOX_VF_REPLY_RECONNECT;
+ break;
+ }
+ if ((flags & ENIC_MAC_ADDR_FLAG_REPLY_MASK) &&
+ !(flags & ENIC_MAC_ADDR_FLAG_SKIPPED))
+ reported_errors++;
+ }
+ if (!status && reported_errors != ret_minor)
+ status = enic_mbox_vf_classify_reply(true, 0,
+ &recovery);
+
+ if (!status)
+ for (i = 0; i < num_addrs; i++)
+ enic->mbox_reply_mac_addrs[i].flags =
+ reply->mac_addr[i].flags;
+ }
+ /* After a malformed reply, the VF cannot trust that its state matches the
+ * PF. VF_NOT_REGISTERED means the PF removed all VF state. Both require a
+ * new registration; an ordinary policy rejection does not.
+ */
+ enic_mbox_vf_recover_reply_locked(enic, recovery);
+ WRITE_ONCE(enic->mbox_reply_status, status);
+ enic_mbox_vf_reply_complete(enic);
+ if (recovery != ENIC_MBOX_VF_REPLY_OK)
+ enic_mbox_vf_kick_recovery(enic);
+}
+
+static void enic_mbox_vf_handle_set_pkt_filter_reply(struct enic *enic,
+ void *payload, u16 msg_len,
+ u64 msg_num)
+{
+ struct enic_mbox_vf_set_pkt_filter_reply_msg *reply = payload;
+ enum enic_mbox_vf_reply_recovery recovery;
+ u16 applied = 0;
+ u16 ret_major = 0;
+ u8 expected;
+ int status;
+
+ if (msg_len < sizeof(*reply)) {
+ status = enic_mbox_vf_classify_reply(true, 0, &recovery);
+ } else {
+ ret_major = le16_to_cpu(reply->reply.ret_major);
+ status = enic_mbox_vf_classify_reply(false, ret_major,
+ &recovery);
+ if (status && (ret_major & ~ENIC_MBOX_ERR_MASK))
+ recovery = ENIC_MBOX_VF_REPLY_RECONNECT;
+ }
+ if (!status)
+ applied = le16_to_cpu(reply->reply.ret_minor);
+
+ if (!enic_mbox_vf_reply_claim(enic,
+ ENIC_MBOX_VF_SET_PKT_FILTER_REPLY,
+ msg_num, &expected))
+ return;
+ enic_mbox_vf_recover_reply_locked(enic, recovery);
+ WRITE_ONCE(enic->mbox_reply_status, status);
+ WRITE_ONCE(enic->mbox_reply_filter_flags, applied);
+ enic_mbox_vf_reply_complete(enic);
+ if (recovery != ENIC_MBOX_VF_REPLY_OK)
+ enic_mbox_vf_kick_recovery(enic);
}
static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload,
@@ -680,6 +905,34 @@ static bool enic_mbox_vf_payload_ok(struct enic *enic, u8 msg_type,
return true;
}
+static void enic_mbox_vf_malformed_msg(struct enic *enic, u8 msg_type,
+ u64 msg_num)
+{
+ u8 expected;
+
+ switch (msg_type) {
+ case ENIC_MBOX_PF_LINK_STATE_NOTIF:
+ case ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF:
+ enic_mbox_vf_require_reconnect(enic);
+ return;
+ case ENIC_MBOX_VF_CAPABILITY_REPLY:
+ case ENIC_MBOX_VF_REGISTER_REPLY:
+ case ENIC_MBOX_VF_UNREGISTER_REPLY:
+ case ENIC_MBOX_VF_ADD_DEL_MAC_REPLY:
+ case ENIC_MBOX_VF_SET_PKT_FILTER_REPLY:
+ break;
+ default:
+ return;
+ }
+
+ if (!enic_mbox_vf_reply_claim(enic, msg_type, msg_num, &expected))
+ return;
+ enic_mbox_vf_mark_reconnect_locked(enic, false);
+ WRITE_ONCE(enic->mbox_reply_status, -EIO);
+ enic_mbox_vf_reply_complete(enic);
+ enic_mbox_vf_kick_recovery(enic);
+}
+
static void enic_mbox_vf_process_msg(struct enic *enic,
struct enic_mbox_hdr *hdr, void *payload,
u16 payload_len)
@@ -691,8 +944,10 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
size_t exp = sizeof(struct enic_mbox_vf_capability_reply_msg);
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
- payload_len, exp))
+ payload_len, exp)) {
+ enic_mbox_vf_malformed_msg(enic, hdr->msg_type, msg_num);
return;
+ }
enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
break;
}
@@ -700,8 +955,10 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
size_t exp = sizeof(struct enic_mbox_vf_register_reply_msg);
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
- payload_len, exp))
+ payload_len, exp)) {
+ enic_mbox_vf_malformed_msg(enic, hdr->msg_type, msg_num);
return;
+ }
enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
break;
}
@@ -709,8 +966,10 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
size_t exp = sizeof(struct enic_mbox_vf_register_reply_msg);
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
- payload_len, exp))
+ payload_len, exp)) {
+ enic_mbox_vf_malformed_msg(enic, hdr->msg_type, msg_num);
return;
+ }
enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
break;
}
@@ -718,11 +977,21 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
size_t exp = sizeof(struct enic_mbox_pf_link_state_notif_msg);
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
- payload_len, exp))
+ payload_len, exp)) {
+ enic_mbox_vf_malformed_msg(enic, hdr->msg_type, msg_num);
return;
+ }
enic_mbox_vf_handle_link_state(enic, payload, msg_num);
break;
}
+ case ENIC_MBOX_VF_ADD_DEL_MAC_REPLY:
+ enic_mbox_vf_handle_add_del_mac_reply(enic, payload,
+ payload_len, msg_num);
+ break;
+ case ENIC_MBOX_VF_SET_PKT_FILTER_REPLY:
+ enic_mbox_vf_handle_set_pkt_filter_reply(enic, payload,
+ payload_len, msg_num);
+ break;
default:
netdev_dbg(enic->netdev,
"MBOX: VF unhandled msg type %u\n",
@@ -762,6 +1031,10 @@ static void enic_mbox_recv_handler(struct enic *enic, void *buf,
netdev_warn(enic->netdev,
"MBOX: invalid msg_len %u (buf len %u)\n",
msg_len, len);
+ if (!enic->vf_state &&
+ le16_to_cpu(hdr->src_vnic_id) == ENIC_MBOX_DST_PF)
+ enic_mbox_vf_malformed_msg(enic, hdr->msg_type,
+ le64_to_cpu(hdr->msg_num));
return;
}
@@ -792,10 +1065,12 @@ int enic_mbox_vf_capability_check(struct enic *enic)
{
struct enic_mbox_vf_capability_msg req = {};
u32 version;
+ int status;
int err;
enic_mbox_vf_request_start(enic);
WRITE_ONCE(enic->pf_cap_version, 0);
+ WRITE_ONCE(enic->mbox_reply_status, 0);
req.version = cpu_to_le32(ENIC_MBOX_CAP_VERSION_1);
err = enic_mbox_vf_send_request(enic,
@@ -809,12 +1084,15 @@ int enic_mbox_vf_capability_check(struct enic *enic)
err = enic_mbox_wait_reply(enic, 3000);
version = READ_ONCE(enic->pf_cap_version);
+ status = READ_ONCE(enic->mbox_reply_status);
enic_mbox_vf_request_finish(enic);
if (err) {
netdev_warn(enic->netdev,
"MBOX: no capability reply from PF\n");
return err;
}
+ if (status)
+ return status;
if (version < ENIC_MBOX_CAP_VERSION_1) {
netdev_warn(enic->netdev,
@@ -829,10 +1107,12 @@ int enic_mbox_vf_capability_check(struct enic *enic)
int enic_mbox_vf_register(struct enic *enic)
{
bool registered;
+ int status;
int err;
enic_mbox_vf_request_start(enic);
WRITE_ONCE(enic->vf_registered, false);
+ WRITE_ONCE(enic->mbox_reply_status, 0);
err = enic_mbox_vf_send_request(enic, ENIC_MBOX_VF_REGISTER_REQUEST,
ENIC_MBOX_VF_REGISTER_REPLY, NULL, 0);
@@ -843,12 +1123,15 @@ int enic_mbox_vf_register(struct enic *enic)
err = enic_mbox_wait_reply(enic, 3000);
registered = READ_ONCE(enic->vf_registered);
+ status = READ_ONCE(enic->mbox_reply_status);
enic_mbox_vf_request_finish(enic);
if (err) {
netdev_warn(enic->netdev,
"MBOX: VF registration with PF timed out\n");
return err;
}
+ if (status)
+ return status;
if (!registered)
return -ENODEV;
@@ -859,15 +1142,18 @@ int enic_mbox_vf_register(struct enic *enic)
int enic_mbox_vf_unregister(struct enic *enic)
{
bool registered;
+ int status;
int err;
if (!READ_ONCE(enic->vf_registered))
return 0;
+
enic_mbox_vf_request_start(enic);
if (!READ_ONCE(enic->vf_registered)) {
enic_mbox_vf_request_finish(enic);
return 0;
}
+ WRITE_ONCE(enic->mbox_reply_status, 0);
err = enic_mbox_vf_send_request(enic,
ENIC_MBOX_VF_UNREGISTER_REQUEST,
@@ -880,14 +1166,182 @@ int enic_mbox_vf_unregister(struct enic *enic)
err = enic_mbox_wait_reply(enic, 3000);
registered = READ_ONCE(enic->vf_registered);
+ status = READ_ONCE(enic->mbox_reply_status);
enic_mbox_vf_request_finish(enic);
if (err)
return err;
+ if (status)
+ return status;
if (registered)
return -EACCES;
return 0;
}
+int enic_mbox_vf_add_del_macs(struct enic *enic,
+ struct enic_mac_addr *macs, u16 num_macs)
+{
+ struct enic_mbox_vf_add_del_mac_msg *req;
+ unsigned int i;
+ int status;
+ int err;
+
+ if (!READ_ONCE(enic->vf_registered) || !enic->has_admin_channel)
+ return -ENODEV;
+ if (!num_macs || num_macs > ENIC_MBOX_MAX_MAC_OPS)
+ return -EINVAL;
+
+ req = kzalloc_flex(*req, mac_addr, num_macs);
+ if (!req)
+ return -ENOMEM;
+
+ req->num_addrs = cpu_to_le16(num_macs);
+ for (i = 0; i < num_macs; i++)
+ req->mac_addr[i] = macs[i];
+
+ enic_mbox_vf_request_start(enic);
+ if (!READ_ONCE(enic->vf_registered) || !enic->has_admin_channel) {
+ err = -ENODEV;
+ } else {
+ spin_lock_bh(&enic->mbox_state_lock);
+ enic->mbox_reply_mac_addrs = macs;
+ enic->mbox_reply_mac_count = num_macs;
+ spin_unlock_bh(&enic->mbox_state_lock);
+ WRITE_ONCE(enic->mbox_reply_status, 0);
+ err = enic_mbox_vf_send_request(enic,
+ ENIC_MBOX_VF_ADD_DEL_MAC_REQUEST,
+ ENIC_MBOX_VF_ADD_DEL_MAC_REPLY,
+ req,
+ struct_size(req, mac_addr,
+ num_macs));
+ }
+ kfree(req);
+ if (err) {
+ spin_lock_bh(&enic->mbox_state_lock);
+ enic->mbox_reply_mac_addrs = NULL;
+ enic->mbox_reply_mac_count = 0;
+ spin_unlock_bh(&enic->mbox_state_lock);
+ enic_mbox_vf_request_abort(enic);
+ return err;
+ }
+
+ err = enic_mbox_wait_reply(enic, 3000);
+ status = READ_ONCE(enic->mbox_reply_status);
+ spin_lock_bh(&enic->mbox_state_lock);
+ enic->mbox_reply_mac_addrs = NULL;
+ enic->mbox_reply_mac_count = 0;
+ spin_unlock_bh(&enic->mbox_state_lock);
+ if (err) {
+ /* The PF may have updated its software ledger before a hardware
+ * failure whose reply was lost. A repeated idempotent operation could
+ * then appear converged while hardware state is stale, so replay from
+ * a fresh registration generation.
+ */
+ enic_mbox_vf_require_reconnect(enic);
+ enic_mbox_vf_request_finish(enic);
+ return err;
+ }
+ enic_mbox_vf_request_finish(enic);
+
+ return status;
+}
+
+int enic_mbox_vf_add_del_mac(struct enic *enic, const u8 *addr, bool add,
+ bool station)
+{
+ struct enic_mac_addr mac = {};
+ u16 flags = 0;
+ int err;
+
+ ether_addr_copy(mac.addr, addr);
+ if (add)
+ flags |= ENIC_MAC_ADDR_FLAG_ADD;
+ if (station)
+ flags |= ENIC_MAC_ADDR_FLAG_STATION;
+ mac.flags = cpu_to_le16(flags);
+
+ err = enic_mbox_vf_add_del_macs(enic, &mac, 1);
+ if (err)
+ return err;
+ if (le16_to_cpu(mac.flags) & ENIC_MAC_ADDR_FLAG_ERROR_MASK)
+ return -EACCES;
+
+ return 0;
+}
+
+int enic_mbox_vf_set_pkt_filter(struct enic *enic, int directed,
+ int multicast, int broadcast,
+ int promisc, int allmulti, u16 *applied_flags)
+{
+ struct enic_mbox_vf_set_pkt_filter_msg req = {};
+ u16 applied;
+ u16 flags = 0;
+ u16 required;
+ int status;
+ int err;
+
+ if (!READ_ONCE(enic->vf_registered) || !enic->has_admin_channel)
+ return -ENODEV;
+
+ if (directed)
+ flags |= CMD_PFILTER_DIRECTED;
+ if (multicast)
+ flags |= CMD_PFILTER_MULTICAST;
+ if (broadcast)
+ flags |= CMD_PFILTER_BROADCAST;
+ if (promisc)
+ flags |= CMD_PFILTER_PROMISCUOUS;
+ if (allmulti)
+ flags |= CMD_PFILTER_ALL_MULTICAST;
+ req.flags = cpu_to_le16(flags);
+
+ enic_mbox_vf_request_start(enic);
+ if (!READ_ONCE(enic->vf_registered) || !enic->has_admin_channel) {
+ enic_mbox_vf_request_abort(enic);
+ return -ENODEV;
+ }
+ WRITE_ONCE(enic->mbox_reply_status, 0);
+ WRITE_ONCE(enic->mbox_reply_filter_flags, 0);
+
+ err = enic_mbox_vf_send_request(enic,
+ ENIC_MBOX_VF_SET_PKT_FILTER_REQUEST,
+ ENIC_MBOX_VF_SET_PKT_FILTER_REPLY,
+ &req, sizeof(req));
+ if (err) {
+ enic_mbox_vf_request_abort(enic);
+ return err;
+ }
+
+ err = enic_mbox_wait_reply(enic, 3000);
+ status = READ_ONCE(enic->mbox_reply_status);
+ if (!err && !status) {
+ applied = READ_ONCE(enic->mbox_reply_filter_flags);
+ /* Directed, multicast, and broadcast are not policy-gated. The PF
+ * may only withhold the two broad receive modes, and may add directed
+ * reception because it is mandatory for a usable VF.
+ */
+ required = (flags | CMD_PFILTER_DIRECTED) &
+ ~(CMD_PFILTER_PROMISCUOUS |
+ CMD_PFILTER_ALL_MULTICAST);
+ if ((applied & ~(flags | CMD_PFILTER_DIRECTED)) ||
+ (applied & required) != required) {
+ netdev_warn(enic->netdev,
+ "MBOX: invalid packet filter reply %#x for request %#x\n",
+ applied, flags);
+ enic_mbox_vf_require_reconnect(enic);
+ status = -EIO;
+ } else if (applied_flags) {
+ *applied_flags = applied;
+ }
+ }
+ if (err)
+ enic_mbox_vf_require_reconnect(enic);
+ enic_mbox_vf_request_finish(enic);
+ if (err)
+ return err;
+
+ return status;
+}
+
void enic_mbox_init(struct enic *enic)
{
bool reinit = enic->mbox_initialized;
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.h b/drivers/net/ethernet/cisco/enic/enic_mbox.h
index 37bc41a900f4..5eca3a25671e 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.h
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.h
@@ -5,6 +5,7 @@
#define _ENIC_MBOX_H_
#include <linux/bits.h>
+#include <linux/if_ether.h>
#include <linux/types.h>
/*
@@ -22,6 +23,12 @@ enum enic_mbox_msg_type {
ENIC_MBOX_VF_UNREGISTER_REPLY = 5,
ENIC_MBOX_PF_LINK_STATE_NOTIF = 6,
ENIC_MBOX_PF_LINK_STATE_ACK = 7,
+ ENIC_MBOX_VF_ADD_DEL_MAC_REQUEST = 10,
+ ENIC_MBOX_VF_ADD_DEL_MAC_REPLY = 11,
+ ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF = 12,
+ ENIC_MBOX_PF_SET_ADMIN_MAC_ACK = 13,
+ ENIC_MBOX_VF_SET_PKT_FILTER_REQUEST = 14,
+ ENIC_MBOX_VF_SET_PKT_FILTER_REPLY = 15,
ENIC_MBOX_MAX
};
@@ -42,6 +49,9 @@ struct enic_mbox_generic_reply {
#define ENIC_MBOX_ERR_GENERIC BIT(0)
#define ENIC_MBOX_ERR_VF_NOT_REGISTERED BIT(1)
#define ENIC_MBOX_ERR_MSG_NOT_SUPPORTED BIT(2)
+#define ENIC_MBOX_ERR_MASK (ENIC_MBOX_ERR_GENERIC | \
+ ENIC_MBOX_ERR_VF_NOT_REGISTERED | \
+ ENIC_MBOX_ERR_MSG_NOT_SUPPORTED)
/* ENIC_MBOX_VF_CAPABILITY_REQUEST / _REPLY */
#define ENIC_MBOX_CAP_VERSION_0 0
@@ -80,6 +90,71 @@ struct enic_mbox_pf_link_state_ack_msg {
struct enic_mbox_generic_reply ack;
};
+/* ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF / _ACK */
+struct enic_mbox_pf_set_admin_mac_notif_msg {
+ u8 mac_addr[ETH_ALEN];
+ __le16 pad;
+};
+
+/* ENIC_MBOX_VF_ADD_DEL_MAC_REQUEST / _REPLY */
+#define ENIC_MAC_ADDR_FLAG_ADD BIT(0)
+#define ENIC_MAC_ADDR_FLAG_STATION BIT(1)
+#define ENIC_MAC_ADDR_FLAG_OVERFLOW BIT(8)
+#define ENIC_MAC_ADDR_FLAG_DUPLICATE BIT(9)
+#define ENIC_MAC_ADDR_FLAG_FAILED BIT(10)
+#define ENIC_MAC_ADDR_FLAG_NOT_FOUND BIT(11)
+#define ENIC_MAC_ADDR_FLAG_ERROR BIT(12)
+#define ENIC_MAC_ADDR_FLAG_NOT_PERMITTED BIT(13)
+#define ENIC_MAC_ADDR_FLAG_INVALID BIT(14)
+#define ENIC_MAC_ADDR_FLAG_SKIPPED BIT(15)
+
+#define ENIC_MAC_ADDR_FLAG_REQUEST_MASK GENMASK(7, 0)
+#define ENIC_MAC_ADDR_FLAG_REPLY_MASK GENMASK(15, 8)
+#define ENIC_MAC_ADDR_FLAG_INDETERMINATE_MASK \
+ (ENIC_MAC_ADDR_FLAG_FAILED | ENIC_MAC_ADDR_FLAG_ERROR)
+#define ENIC_MAC_ADDR_FLAG_PERMANENT_MASK \
+ (ENIC_MAC_ADDR_FLAG_OVERFLOW | ENIC_MAC_ADDR_FLAG_NOT_PERMITTED | \
+ ENIC_MAC_ADDR_FLAG_INVALID)
+#define ENIC_MAC_ADDR_FLAG_ERROR_MASK (ENIC_MAC_ADDR_FLAG_OVERFLOW | \
+ ENIC_MAC_ADDR_FLAG_FAILED | \
+ ENIC_MAC_ADDR_FLAG_ERROR | \
+ ENIC_MAC_ADDR_FLAG_NOT_PERMITTED | \
+ ENIC_MAC_ADDR_FLAG_INVALID | \
+ ENIC_MAC_ADDR_FLAG_SKIPPED)
+
+/* The protocol permits replacing all perfect filters and the station address
+ * in one request: one delete and one add operation for each address.
+ */
+#define ENIC_MBOX_MAX_MAC_OPS 130
+
+struct enic_mac_addr {
+ u8 addr[ETH_ALEN];
+ __le16 flags;
+};
+
+struct enic_mbox_vf_add_del_mac_msg {
+ __le16 num_addrs;
+ __le16 pad;
+ struct enic_mac_addr mac_addr[];
+};
+
+struct enic_mbox_vf_add_del_mac_reply_msg {
+ struct enic_mbox_generic_reply reply;
+ __le16 num_addrs;
+ __le16 pad;
+ struct enic_mac_addr mac_addr[];
+};
+
+/* ENIC_MBOX_VF_SET_PKT_FILTER_REQUEST / _REPLY */
+struct enic_mbox_vf_set_pkt_filter_msg {
+ __le16 flags;
+ __le16 pad;
+};
+
+struct enic_mbox_vf_set_pkt_filter_reply_msg {
+ struct enic_mbox_generic_reply reply;
+};
+
#define ENIC_MBOX_DST_PF 0xFFFF
struct enic;
@@ -95,5 +170,12 @@ void enic_mbox_vf_require_reconnect(struct enic *enic);
int enic_mbox_vf_capability_check(struct enic *enic);
int enic_mbox_vf_register(struct enic *enic);
int enic_mbox_vf_unregister(struct enic *enic);
+int enic_mbox_vf_add_del_macs(struct enic *enic,
+ struct enic_mac_addr *macs, u16 num_macs);
+int enic_mbox_vf_add_del_mac(struct enic *enic, const u8 *addr, bool add,
+ bool station);
+int enic_mbox_vf_set_pkt_filter(struct enic *enic, int directed, int multicast,
+ int broadcast, int promisc, int allmulti,
+ u16 *applied_flags);
#endif /* _ENIC_MBOX_H_ */
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 5/6] enic: manage V2 VF station and administrative MAC
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
` (3 preceding siblings ...)
2026-09-29 18:58 ` [PATCH net-next v2 4/6] enic: validate V2 VF configuration replies Satish Kharat
@ 2026-09-29 18:58 ` Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
2026-09-29 18:58 ` [PATCH net-next v2 6/6] enic: configure V2 VF receive mode over mailbox Satish Kharat
5 siblings, 1 reply; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
Use the V2 mailbox MAC operation for the VF station address and keep that
address separate from netdev secondary-unicast synchronization. Replace a
station address with one mailbox request containing a delete followed by an
add. If the PF reports that only part of the replacement was applied,
require the VF to register again before accepting traffic.
Treat the PF administrative MAC as authoritative. Process notifications
outside receive dispatch, preserve a delegated VF-selected address, refresh
the policy after every registration, and protect VF worker shutdown and
restart against teardown and recovery.
Reject a VF MAC change which conflicts with a cached nonzero administrative
MAC even while the datapath is down. During registration, repair an actual
device-address mismatch even when the administrative policy itself did not
change since the previous registration.
Install the station address during open and remove it during an ordinary
stop. At reset and removal boundaries, rely on VF_UNREGISTER or the next VF
registration to clear the old state.
Assisted-by: LLM
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 17 +
drivers/net/ethernet/cisco/enic/enic_admin.c | 2 +
drivers/net/ethernet/cisco/enic/enic_dev.c | 11 +
drivers/net/ethernet/cisco/enic/enic_dev.h | 1 +
drivers/net/ethernet/cisco/enic/enic_main.c | 578 ++++++++++++++++++++++++++-
drivers/net/ethernet/cisco/enic/enic_mbox.c | 30 ++
6 files changed, 635 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index ef8332138c5a..1e369e6704e1 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -345,6 +345,9 @@ struct enic {
spinlock_t vf_link_state_lock;
enum enic_vf_link_state vf_link_state;
bool vf_link_running;
+ /* Last station address which may still be installed at the PF. */
+ u8 vf_station_addr[ETH_ALEN] __aligned(2);
+ bool vf_station_addr_valid;
/* Tracks a completely opened V2 VF datapath. An internal reset can stop
* it while netif_running() remains true, then fail before reopen.
*/
@@ -365,6 +368,16 @@ struct enic {
struct list_head vf_ack_list;
struct work_struct vf_ack_work;
unsigned int vf_ack_count;
+ struct delayed_work vf_admin_mac_work;
+ spinlock_t vf_admin_mac_lock; /* protects pending admin MAC */
+ u8 vf_admin_mac[ETH_ALEN];
+ u8 vf_admin_mac_random_addr[ETH_ALEN];
+ bool vf_admin_mac_pending;
+ bool vf_admin_mac_random_valid;
+ bool vf_admin_mac_work_enabled;
+ bool vf_admin_mac_recovery_attempted;
+ u32 vf_admin_mac_generation;
+ u8 vf_admin_mac_retries;
u64 mbox_expected_msg_num;
u8 mbox_expected_reply;
int mbox_reply_status;
@@ -498,6 +511,10 @@ static inline int enic_dma_map_check(struct enic *enic, dma_addr_t dma_addr)
}
void enic_reset_addr_lists(struct enic *enic);
+void enic_vf_admin_mac_notify(struct enic *enic, const u8 *addr);
+void enic_vf_admin_mac_quiesce(struct enic *enic);
+void enic_vf_admin_mac_rearm(struct enic *enic);
+void enic_vf_admin_mac_purge(struct enic *enic);
int enic_sriov_enabled(struct enic *enic);
int enic_is_valid_vf(struct enic *enic, int vf);
int enic_is_dynamic(struct enic *enic);
diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c
index 30c0a5c89a2d..5e52d4da8d3c 100644
--- a/drivers/net/ethernet/cisco/enic/enic_admin.c
+++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
@@ -641,6 +641,8 @@ void enic_admin_channel_close(struct enic *enic)
{
int err;
+ if (enic_is_sriov_vf_v2(enic))
+ enic_vf_admin_mac_quiesce(enic);
if (!enic->has_admin_channel)
return;
diff --git a/drivers/net/ethernet/cisco/enic/enic_dev.c b/drivers/net/ethernet/cisco/enic/enic_dev.c
index 659787f73cf1..48c1ca9b40fb 100644
--- a/drivers/net/ethernet/cisco/enic/enic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/enic_dev.c
@@ -32,6 +32,17 @@ int enic_dev_stats_dump(struct enic *enic, struct vnic_stats **vstats)
return err;
}
+int enic_dev_get_mac_addr(struct enic *enic, u8 *mac_addr)
+{
+ int err;
+
+ spin_lock_bh(&enic->devcmd_lock);
+ err = vnic_dev_get_mac_addr(enic->vdev, mac_addr);
+ spin_unlock_bh(&enic->devcmd_lock);
+
+ return err;
+}
+
int enic_dev_add_station_addr(struct enic *enic)
{
int err;
diff --git a/drivers/net/ethernet/cisco/enic/enic_dev.h b/drivers/net/ethernet/cisco/enic/enic_dev.h
index 698d0cb02064..66d0e51681f5 100644
--- a/drivers/net/ethernet/cisco/enic/enic_dev.h
+++ b/drivers/net/ethernet/cisco/enic/enic_dev.h
@@ -26,6 +26,7 @@
int enic_dev_fw_info(struct enic *enic, struct vnic_devcmd_fw_info **fw_info);
int enic_dev_stats_dump(struct enic *enic, struct vnic_stats **vstats);
+int enic_dev_get_mac_addr(struct enic *enic, u8 *mac_addr);
int enic_dev_add_station_addr(struct enic *enic);
int enic_dev_del_station_addr(struct enic *enic);
int enic_dev_packet_filter(struct enic *enic, int directed, int multicast,
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 28738bd14fa7..2132bfa9c8d3 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -89,6 +89,9 @@ MODULE_DEVICE_TABLE(pci, enic_id_table);
#define ENIC_LARGE_PKT_THRESHOLD 1000
#define ENIC_MAX_COALESCE_TIMERS 10
+#define ENIC_VF_ADMIN_MAC_MAX_RETRIES 4
+#define ENIC_VF_ADMIN_MAC_RETRY_MS 100
+#define ENIC_VF_ADMIN_MAC_REFRESH_RETRIES 3
/* Interrupt moderation table, which will be used to decide the
* coalescing timer values
* {rx_rate in Mbps, mapping percentage of the range}
@@ -1040,8 +1043,10 @@ void enic_reset_addr_lists(struct enic *enic)
{
struct net_device *netdev = enic->netdev;
+ netif_addr_lock_bh(netdev);
__dev_uc_unsync(netdev, NULL);
__dev_mc_unsync(netdev, NULL);
+ netif_addr_unlock_bh(netdev);
enic->mc_count = 0;
enic->uc_count = 0;
@@ -1065,6 +1070,461 @@ static int enic_set_mac_addr(struct net_device *netdev, char *addr)
return 0;
}
+static void enic_vf_station_recovery_required(struct enic *enic)
+{
+ enic_mbox_vf_require_reconnect(enic);
+}
+
+static void enic_vf_station_addr_set(struct enic *enic, const u8 *addr)
+{
+ ether_addr_copy(enic->vf_station_addr, addr);
+ enic->vf_station_addr_valid = true;
+}
+
+static int enic_vf_keep_nonstation_sync(struct net_device *netdev,
+ const u8 *addr)
+{
+ struct enic *enic = netdev_priv(netdev);
+
+ if (!ether_addr_equal(addr, enic->vf_station_addr))
+ return -ENOENT;
+ if (WARN_ON_ONCE(!enic->uc_count))
+ return 0;
+ enic->uc_count--;
+
+ return 0;
+}
+
+static void enic_vf_station_sync_reset(struct enic *enic)
+{
+ if (!enic->vf_station_addr_valid)
+ return;
+
+ /* The PF keys its MAC ledger by address. A station entry is also the
+ * receive filter for that address and must not retain a second core
+ * synchronization reference which could later delete the station.
+ */
+ netif_addr_lock_bh(enic->netdev);
+ __dev_uc_unsync(enic->netdev, enic_vf_keep_nonstation_sync);
+ netif_addr_unlock_bh(enic->netdev);
+}
+
+static int enic_vf_station_addr_del(struct enic *enic)
+{
+ int err;
+
+ if (!enic->vf_station_addr_valid)
+ return 0;
+
+ err = enic_mbox_vf_add_del_mac(enic, enic->vf_station_addr,
+ false, true);
+ if (!err) {
+ enic->vf_station_addr_valid = false;
+ } else if (READ_ONCE(enic->mbox_tx_poisoned)) {
+ enic_mbox_vf_link_state_set_running(enic, false);
+ } else if (err == -EACCES) {
+ /* A definitive rejected DELETE leaves an unrequested station
+ * filter installed. Fresh registration is the cleanup boundary.
+ */
+ enic_vf_station_recovery_required(enic);
+ }
+
+ return err;
+}
+
+/* End every replacement with an equal-address DEL+ADD pair. A bare ADD could
+ * report DUPLICATE for a secondary exact filter without applying the current
+ * station-address policy. If another station is tracked, delete it first in
+ * the same compound request.
+ */
+static int enic_vf_station_addr_replace(struct enic *enic, const u8 *addr)
+{
+ struct enic_mac_addr macs[3] = {};
+ bool all_deletes_skipped = true;
+ bool deletes_converged = true;
+ u16 num_macs = 0;
+ unsigned int i;
+ u16 result;
+ int err;
+
+ if (enic->vf_station_addr_valid &&
+ !ether_addr_equal(enic->vf_station_addr, addr)) {
+ ether_addr_copy(macs[num_macs].addr, enic->vf_station_addr);
+ macs[num_macs++].flags =
+ cpu_to_le16(ENIC_MAC_ADDR_FLAG_STATION);
+ }
+ ether_addr_copy(macs[num_macs].addr, addr);
+ macs[num_macs++].flags = cpu_to_le16(ENIC_MAC_ADDR_FLAG_STATION);
+ ether_addr_copy(macs[num_macs].addr, addr);
+ macs[num_macs++].flags = cpu_to_le16(ENIC_MAC_ADDR_FLAG_ADD |
+ ENIC_MAC_ADDR_FLAG_STATION);
+
+ err = enic_mbox_vf_add_del_macs(enic, macs, num_macs);
+ if (err) {
+ if (READ_ONCE(enic->mbox_tx_poisoned))
+ enic_mbox_vf_link_state_set_running(enic, false);
+ return err;
+ }
+
+ for (i = 0; i < num_macs - 1; i++) {
+ result = le16_to_cpu(macs[i].flags) &
+ ENIC_MAC_ADDR_FLAG_REPLY_MASK;
+ if (result != ENIC_MAC_ADDR_FLAG_SKIPPED)
+ all_deletes_skipped = false;
+ if (result && result != ENIC_MAC_ADDR_FLAG_NOT_FOUND)
+ deletes_converged = false;
+ }
+ result = le16_to_cpu(macs[num_macs - 1].flags) &
+ ENIC_MAC_ADDR_FLAG_REPLY_MASK;
+ if (deletes_converged && !result)
+ return 0;
+
+ /* A policy-rejected ADD can be reported with all preceding DELETEs
+ * skipped. No operation changed state in that coherent result tuple.
+ */
+ if (all_deletes_skipped && result) {
+ if (result & (ENIC_MAC_ADDR_FLAG_DUPLICATE |
+ ENIC_MAC_ADDR_FLAG_PERMANENT_MASK))
+ return -EACCES;
+ return -EIO;
+ }
+
+ /* After any other partial or contradictory result, the VF does not know
+ * which station address the PF kept. Re-register before accepting traffic.
+ */
+ enic_vf_station_recovery_required(enic);
+ for (i = 0; i < num_macs; i++)
+ if (le16_to_cpu(macs[i].flags) &
+ ENIC_MAC_ADDR_FLAG_PERMANENT_MASK)
+ return -EACCES;
+
+ return -EIO;
+}
+
+static void enic_vf_admin_mac_cache_selected(struct enic *enic,
+ const u8 *addr)
+{
+ /* A zero administrative policy delegates the operational address to
+ * the VF. Preserve a successful user selection across reconnects.
+ */
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ if (is_zero_ether_addr(enic->vf_admin_mac)) {
+ ether_addr_copy(enic->vf_admin_mac_random_addr, addr);
+ enic->vf_admin_mac_random_valid = true;
+ }
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+}
+
+static void enic_vf_admin_mac_work(struct work_struct *work)
+{
+ struct enic *enic = container_of(to_delayed_work(work), struct enic,
+ vf_admin_mac_work);
+ u8 policy[ETH_ALEN];
+ u8 selected[ETH_ALEN];
+ unsigned long delay = 0;
+ u32 generation = 0;
+ bool changed;
+ bool mutated = false;
+ bool reschedule = false;
+ bool station_installed = false;
+ bool stale = false;
+ bool zero_policy;
+ bool pending;
+ int err = 0;
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ pending = enic->vf_admin_mac_pending &&
+ enic->vf_admin_mac_work_enabled;
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ if (!pending)
+ return;
+
+ /* Teardown owns RTNL while synchronously cancelling this work. Do not
+ * block that owner.
+ */
+ if (!rtnl_trylock()) {
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ if (enic->vf_admin_mac_pending &&
+ enic->vf_admin_mac_work_enabled)
+ mod_delayed_work(system_wq, &enic->vf_admin_mac_work,
+ msecs_to_jiffies(10));
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ return;
+ }
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ pending = enic->vf_admin_mac_pending &&
+ enic->vf_admin_mac_work_enabled;
+ if (pending) {
+ ether_addr_copy(policy, enic->vf_admin_mac);
+ generation = enic->vf_admin_mac_generation;
+ zero_policy = is_zero_ether_addr(policy);
+ if (zero_policy && enic->vf_admin_mac_random_valid)
+ ether_addr_copy(selected,
+ enic->vf_admin_mac_random_addr);
+ }
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ if (!pending)
+ goto unlock;
+
+ if (zero_policy && !READ_ONCE(enic->vf_admin_mac_random_valid)) {
+ eth_random_addr(selected);
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ if (!enic->vf_admin_mac_pending ||
+ !enic->vf_admin_mac_work_enabled ||
+ enic->vf_admin_mac_generation != generation ||
+ !is_zero_ether_addr(enic->vf_admin_mac)) {
+ stale = true;
+ } else if (enic->vf_admin_mac_random_valid) {
+ ether_addr_copy(selected,
+ enic->vf_admin_mac_random_addr);
+ } else {
+ ether_addr_copy(enic->vf_admin_mac_random_addr, selected);
+ enic->vf_admin_mac_random_valid = true;
+ }
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ if (stale)
+ goto unlock;
+ } else if (!zero_policy) {
+ ether_addr_copy(selected, policy);
+ }
+
+ if (!netif_device_present(enic->netdev) ||
+ !READ_ONCE(enic->vf_registered)) {
+ err = -EAGAIN;
+ goto unlock;
+ }
+
+ /* A nonzero notification describes a station address the PF has already
+ * installed. Zero delegates selection to the VF, which registers the
+ * selected random address through the policy-safe replacement request.
+ */
+ if (!READ_ONCE(enic->vf_datapath_open)) {
+ station_installed = !zero_policy &&
+ (!enic->vf_station_addr_valid ||
+ ether_addr_equal(enic->vf_station_addr, selected));
+ } else if (zero_policy) {
+ mutated = true;
+ err = enic_vf_station_addr_replace(enic, selected);
+ station_installed = !err;
+ } else if (enic->vf_station_addr_valid &&
+ !ether_addr_equal(enic->vf_station_addr, selected)) {
+ mutated = true;
+ err = enic_vf_station_addr_del(enic);
+ station_installed = !err;
+ } else {
+ station_installed = true;
+ }
+ if (err)
+ goto unlock;
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ if (!enic->vf_admin_mac_pending ||
+ !enic->vf_admin_mac_work_enabled ||
+ enic->vf_admin_mac_generation != generation) {
+ stale = true;
+ /* If policy changed while the request was running, the completed
+ * station change may no longer match it. Record recovery while holding
+ * vf_admin_mac_lock, which also protects VF worker shutdown. Teardown
+ * therefore either suppresses the update or sees it first, and already
+ * uses unregister/register to clear old state.
+ */
+ if (mutated && enic->vf_admin_mac_work_enabled)
+ enic_vf_station_recovery_required(enic);
+ } else {
+ enic->vf_admin_mac_pending = false;
+ enic->vf_admin_mac_retries = 0;
+ enic->vf_admin_mac_recovery_attempted = false;
+ if (!zero_policy)
+ enic->vf_admin_mac_random_valid = false;
+ }
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ if (stale)
+ goto unlock;
+
+ changed = !ether_addr_equal(enic->netdev->dev_addr, selected);
+ enic_vf_station_sync_reset(enic);
+ eth_hw_addr_set(enic->netdev, selected);
+ if (!zero_policy || changed)
+ enic->netdev->addr_assign_type = zero_policy ?
+ NET_ADDR_RANDOM : NET_ADDR_SET;
+ ether_addr_copy(enic->mac_addr, selected);
+ if (station_installed)
+ enic_vf_station_addr_set(enic, selected);
+ enic_vf_station_sync_reset(enic);
+ if (enic->netdev->reg_state == NETREG_REGISTERED && changed)
+ call_netdevice_notifiers(NETDEV_CHANGEADDR, enic->netdev);
+ netdev_info(enic->netdev, "MBOX: admin MAC set to %pM\n", selected);
+
+unlock:
+ rtnl_unlock();
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ if (err && enic->vf_admin_mac_pending &&
+ enic->vf_admin_mac_work_enabled &&
+ generation == enic->vf_admin_mac_generation &&
+ !READ_ONCE(enic->mbox_send_disabled)) {
+ if (enic->vf_admin_mac_retries <
+ ENIC_VF_ADMIN_MAC_MAX_RETRIES) {
+ enic->vf_admin_mac_retries++;
+ reschedule = true;
+ delay = msecs_to_jiffies(ENIC_VF_ADMIN_MAC_RETRY_MS);
+ } else if (!enic->vf_admin_mac_recovery_attempted) {
+ enic->vf_admin_mac_recovery_attempted = true;
+ /* vf_admin_mac_lock also protects VF worker shutdown. */
+ enic_vf_station_recovery_required(enic);
+ }
+ }
+ if (enic->vf_admin_mac_pending &&
+ enic->vf_admin_mac_work_enabled &&
+ generation != enic->vf_admin_mac_generation) {
+ reschedule = true;
+ delay = 0;
+ }
+ if (enic->vf_admin_mac_pending && reschedule)
+ mod_delayed_work(system_wq, &enic->vf_admin_mac_work, delay);
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+}
+
+void enic_vf_admin_mac_notify(struct enic *enic, const u8 *addr)
+{
+ bool new_policy;
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ new_policy = !enic->vf_admin_mac_pending ||
+ !ether_addr_equal(enic->vf_admin_mac, addr);
+ if (new_policy) {
+ ether_addr_copy(enic->vf_admin_mac, addr);
+ if (!is_zero_ether_addr(addr))
+ enic->vf_admin_mac_random_valid = false;
+ enic->vf_admin_mac_pending = true;
+ enic->vf_admin_mac_generation++;
+ enic->vf_admin_mac_retries = 0;
+ enic->vf_admin_mac_recovery_attempted = false;
+ }
+ if (new_policy && enic->vf_admin_mac_work_enabled)
+ mod_delayed_work(system_wq, &enic->vf_admin_mac_work, 0);
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+}
+
+void enic_vf_admin_mac_quiesce(struct enic *enic)
+{
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ enic->vf_admin_mac_work_enabled = false;
+ enic->vf_admin_mac_generation++;
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ cancel_delayed_work_sync(&enic->vf_admin_mac_work);
+}
+
+void enic_vf_admin_mac_rearm(struct enic *enic)
+{
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ enic->vf_admin_mac_work_enabled = true;
+ if (enic->vf_admin_mac_pending)
+ mod_delayed_work(system_wq, &enic->vf_admin_mac_work, 0);
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+}
+
+void enic_vf_admin_mac_purge(struct enic *enic)
+{
+ enic_vf_admin_mac_quiesce(enic);
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ enic->vf_admin_mac_pending = false;
+ enic->vf_admin_mac_random_valid = false;
+ enic->vf_admin_mac_recovery_attempted = false;
+ enic->vf_admin_mac_retries = 0;
+ enic->vf_admin_mac_generation++;
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+}
+
+/* CMD_GET_MAC_ADDR is the PF-owned policy store. Re-read it after REGISTER so
+ * a lost notification cannot leave the VF using stale administrative policy.
+ */
+static int enic_vf_admin_mac_refresh(struct enic *enic)
+{
+ u8 previous_policy[ETH_ALEN];
+ u8 random_addr[ETH_ALEN];
+ u8 selected[ETH_ALEN];
+ u8 policy[ETH_ALEN];
+ u32 generation;
+ bool apply_policy;
+ bool changed;
+ bool pending;
+ bool policy_changed;
+ bool zero_policy;
+ unsigned int attempt;
+ int err;
+
+ for (attempt = 0; attempt < ENIC_VF_ADMIN_MAC_REFRESH_RETRIES;
+ attempt++) {
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ generation = enic->vf_admin_mac_generation;
+ pending = enic->vf_admin_mac_pending;
+ ether_addr_copy(previous_policy, enic->vf_admin_mac);
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+
+ err = enic_dev_get_mac_addr(enic, policy);
+ if (err)
+ return err;
+ if (is_multicast_ether_addr(policy))
+ return -EADDRNOTAVAIL;
+
+ zero_policy = is_zero_ether_addr(policy);
+ if (zero_policy)
+ eth_random_addr(random_addr);
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ if (enic->vf_admin_mac_generation != generation) {
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ continue;
+ }
+
+ policy_changed = !ether_addr_equal(previous_policy, policy);
+ apply_policy = pending || policy_changed ||
+ !is_valid_ether_addr(enic->netdev->dev_addr) ||
+ (!zero_policy &&
+ !ether_addr_equal(enic->netdev->dev_addr, policy));
+ if (!apply_policy) {
+ ether_addr_copy(selected, enic->netdev->dev_addr);
+ if (zero_policy) {
+ ether_addr_copy(enic->vf_admin_mac_random_addr,
+ selected);
+ enic->vf_admin_mac_random_valid = true;
+ }
+ } else if (zero_policy && enic->vf_admin_mac_random_valid) {
+ ether_addr_copy(selected,
+ enic->vf_admin_mac_random_addr);
+ } else if (zero_policy) {
+ ether_addr_copy(selected, random_addr);
+ ether_addr_copy(enic->vf_admin_mac_random_addr, selected);
+ enic->vf_admin_mac_random_valid = true;
+ } else {
+ ether_addr_copy(selected, policy);
+ enic->vf_admin_mac_random_valid = false;
+ }
+ ether_addr_copy(enic->vf_admin_mac, policy);
+ enic->vf_admin_mac_pending = false;
+ enic->vf_admin_mac_recovery_attempted = false;
+ enic->vf_admin_mac_retries = 0;
+ enic->vf_admin_mac_generation++;
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+
+ changed = !ether_addr_equal(enic->netdev->dev_addr, selected);
+ eth_hw_addr_set(enic->netdev, selected);
+ if (!zero_policy || changed)
+ enic->netdev->addr_assign_type = zero_policy ?
+ NET_ADDR_RANDOM : NET_ADDR_SET;
+ ether_addr_copy(enic->mac_addr, selected);
+ if (changed && enic->netdev->reg_state == NETREG_REGISTERED)
+ call_netdevice_notifiers(NETDEV_CHANGEADDR,
+ enic->netdev);
+
+ return 0;
+ }
+
+ return -EAGAIN;
+}
+
static int enic_set_mac_address_dynamic(struct net_device *netdev, void *p)
{
struct enic *enic = netdev_priv(netdev);
@@ -1072,6 +1532,52 @@ static int enic_set_mac_address_dynamic(struct net_device *netdev, void *p)
char *addr = saddr->sa_data;
int err;
+ if (enic_is_sriov_vf_v2(enic)) {
+ if (!is_valid_ether_addr(addr))
+ return -EADDRNOTAVAIL;
+
+ spin_lock_bh(&enic->vf_admin_mac_lock);
+ err = is_valid_ether_addr(enic->vf_admin_mac) &&
+ !ether_addr_equal(enic->vf_admin_mac, addr);
+ spin_unlock_bh(&enic->vf_admin_mac_lock);
+ if (err)
+ return -EPERM;
+ if (ether_addr_equal(addr, netdev->dev_addr))
+ return 0;
+
+ if (enic->vf_datapath_open &&
+ !READ_ONCE(enic->vf_registered))
+ return -ENODEV;
+
+ /* An internal reset keeps IFF_UP set while its failed reopen leaves
+ * the datapath closed. Cache the requested address in that state; the
+ * next successful open installs it together with the datapath.
+ */
+ if (!enic->vf_datapath_open) {
+ err = enic_set_mac_addr(netdev, addr);
+ if (!err)
+ enic_vf_admin_mac_cache_selected(enic, addr);
+ return err;
+ }
+
+ /* Keep the old software address visible until the complete station
+ * replacement proves convergence.
+ */
+ err = enic_vf_station_addr_replace(enic, addr);
+ if (err)
+ return err;
+
+ enic_vf_station_sync_reset(enic);
+ err = enic_set_mac_addr(netdev, addr);
+ if (!err) {
+ enic_vf_station_addr_set(enic, addr);
+ enic_vf_station_sync_reset(enic);
+ enic_vf_admin_mac_cache_selected(enic, addr);
+ }
+
+ return err;
+ }
+
if (netif_running(enic->netdev)) {
err = enic_dev_del_station_addr(enic);
if (err)
@@ -1726,6 +2232,7 @@ static int enic_admin_chan_reopen(struct enic *enic);
static int enic_open(struct net_device *netdev)
{
struct enic *enic = netdev_priv(netdev);
+ bool vf_mac_added = false;
unsigned int i;
int err, ret;
unsigned int max_pkt_len = netdev->mtu + VLAN_ETH_HLEN;
@@ -1807,6 +2314,25 @@ static int enic_open(struct net_device *netdev)
if (!enic_is_dynamic(enic) && !enic_is_sriov_vf(enic))
enic_dev_add_station_addr(enic);
+ if (enic_is_sriov_vf_v2(enic)) {
+ if (!READ_ONCE(enic->vf_registered)) {
+ netdev_err(netdev, "VF is not registered with its PF\n");
+ err = -ENODEV;
+ goto err_out_disable_wq;
+ }
+
+ err = enic_vf_station_addr_replace(enic, netdev->dev_addr);
+ if (err) {
+ netdev_err(netdev,
+ "Failed to register VF station address: %d\n",
+ err);
+ goto err_out_disable_wq;
+ }
+ enic_vf_station_addr_set(enic, netdev->dev_addr);
+ enic_vf_station_sync_reset(enic);
+ vf_mac_added = true;
+ }
+
enic_set_rx_mode(netdev);
netif_tx_wake_all_queues(netdev);
@@ -1864,6 +2390,15 @@ static int enic_open(struct net_device *netdev)
for (i = 0; i < enic->wq_count; i++)
napi_disable(&enic->napi[enic_cq_wq(enic, i)]);
netif_tx_disable(netdev);
+err_out_disable_wq:
+ if (vf_mac_added) {
+ ret = enic_vf_station_addr_del(enic);
+ if (ret) {
+ netdev_warn(netdev,
+ "Failed to remove VF station address during open rollback: %d\n",
+ ret);
+ }
+ }
if (!enic_is_dynamic(enic) && !enic_is_sriov_vf(enic))
enic_dev_del_station_addr(enic);
for (i = 0; i < enic->wq_count; i++)
@@ -1898,7 +2433,6 @@ static int __enic_stop(struct net_device *netdev, bool remove_vf_station)
*/
if (enic_is_sriov_vf_v2(enic) && !enic->vf_datapath_open)
return 0;
- (void)remove_vf_station;
for (i = 0; i < enic->intr_count; i++) {
vnic_intr_mask(&enic->intr[i]);
@@ -1923,6 +2457,15 @@ static int __enic_stop(struct net_device *netdev, bool remove_vf_station)
for (i = 0; i < enic->wq_count; i++)
napi_disable(&enic->napi[enic_cq_wq(enic, i)]);
netif_tx_disable(netdev);
+ if (remove_vf_station && enic_is_sriov_vf_v2(enic) &&
+ READ_ONCE(enic->vf_registered)) {
+ err = enic_vf_station_addr_del(enic);
+ if (err) {
+ netdev_warn(netdev,
+ "Failed to remove VF station address: %d\n",
+ err);
+ }
+ }
if (!enic_is_dynamic(enic) && !enic_is_sriov_vf(enic))
enic_dev_del_station_addr(enic);
@@ -2321,9 +2864,18 @@ static int enic_admin_chan_reopen(struct enic *enic)
return err;
}
enic_reset_addr_lists(enic);
+ enic->vf_station_addr_valid = false;
+ err = enic_vf_admin_mac_refresh(enic);
+ if (err) {
+ netdev_err(enic->netdev,
+ "Failed to refresh VF admin MAC after reset: %d\n",
+ err);
+ enic_admin_channel_close(enic);
+ return err;
+ }
/* Capability negotiation and registration establish a new protocol
- * generation. RX remains quarantined until enic_open() replays the
- * station and receive policy.
+ * generation after authoritative MAC policy is refreshed. RX remains
+ * quarantined until enic_open() replays station and receive policy.
*/
spin_lock_bh(&enic->mbox_state_lock);
if (enic->vf_mbox_fault_generation != recovery_generation ||
@@ -2339,6 +2891,7 @@ static int enic_admin_chan_reopen(struct enic *enic)
enic_admin_channel_close(enic);
return err;
}
+ enic_vf_admin_mac_rearm(enic);
} else {
/* The link came back up during enic_open() above while MBOX
* sends were still disabled (channel not yet reopened), so that
@@ -3333,6 +3886,9 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
* cancel_work_sync()) would otherwise act on an uninitialised work.
*/
INIT_WORK(&enic->link_notify_work, enic_link_notify_work_handler);
+ spin_lock_init(&enic->vf_admin_mac_lock);
+ INIT_DELAYED_WORK(&enic->vf_admin_mac_work,
+ enic_vf_admin_mac_work);
/* V2 VF: open admin channel and register with PF.
* Must happen before register_netdev so the VF is fully
@@ -3362,6 +3918,12 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
"MBOX VF registration failed: %d\n", err);
goto err_out_admin_close;
}
+ err = enic_vf_admin_mac_refresh(enic);
+ if (err) {
+ dev_err(dev,
+ "MBOX VF admin MAC refresh failed: %d\n", err);
+ goto err_out_admin_close;
+ }
}
netif_set_real_num_tx_queues(netdev, enic->wq_count);
@@ -3487,6 +4049,8 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
dev_err(dev, "Cannot register net device, aborting\n");
goto err_out_admin_close;
}
+ if (enic_is_sriov_vf_v2(enic))
+ enic_vf_admin_mac_rearm(enic);
return 0;
@@ -3542,9 +4106,13 @@ static void enic_remove(struct pci_dev *pdev)
/* Close the admin channel and unregister from the PF before
* unregister_netdev() to prevent a late PF notification from
- * touching a netdev that is being torn down.
+ * touching a netdev that is being torn down. VF_UNREGISTER is the
+ * protocol teardown operation: the PF removes all VF-requested
+ * configuration, including the station address, before replying.
*/
if (enic_is_sriov_vf_v2(enic)) {
+ enic_vf_admin_mac_quiesce(enic);
+
if (READ_ONCE(enic->vf_registered)) {
int unreg_err = enic_mbox_vf_unregister(enic);
@@ -3563,6 +4131,8 @@ static void enic_remove(struct pci_dev *pdev)
* enic_link_check() scheduled it just as SR-IOV was disabled.
*/
cancel_work_sync(&enic->link_notify_work);
+ if (enic_is_sriov_vf_v2(enic))
+ enic_vf_admin_mac_purge(enic);
#ifdef CONFIG_PCI_IOV
if (enic_sriov_enabled(enic)) {
if (enic->vf_type == ENIC_VF_TYPE_V2)
diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
index 265b58cd39be..1fd2b23e2d8f 100644
--- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
+++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
@@ -860,6 +860,25 @@ static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload,
ret_major);
}
+static void enic_mbox_vf_handle_admin_mac(struct enic *enic, void *payload,
+ u64 msg_num)
+{
+ struct enic_mbox_pf_set_admin_mac_notif_msg *notif = payload;
+ u16 ret_major = 0;
+
+ if (is_multicast_ether_addr(notif->mac_addr)) {
+ netdev_warn(enic->netdev,
+ "MBOX: rejecting multicast admin MAC %pM\n",
+ notif->mac_addr);
+ ret_major = ENIC_MBOX_ERR_GENERIC;
+ } else {
+ enic_vf_admin_mac_notify(enic, notif->mac_addr);
+ }
+
+ enic_mbox_vf_queue_ack(enic, ENIC_MBOX_PF_SET_ADMIN_MAC_ACK,
+ msg_num, ret_major);
+}
+
void enic_mbox_vf_link_state_reset(struct enic *enic)
{
spin_lock_bh(&enic->vf_link_state_lock);
@@ -984,6 +1003,17 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
enic_mbox_vf_handle_link_state(enic, payload, msg_num);
break;
}
+ case ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF: {
+ size_t exp = sizeof(struct enic_mbox_pf_set_admin_mac_notif_msg);
+
+ if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
+ payload_len, exp)) {
+ enic_mbox_vf_malformed_msg(enic, hdr->msg_type, msg_num);
+ return;
+ }
+ enic_mbox_vf_handle_admin_mac(enic, payload, msg_num);
+ break;
+ }
case ENIC_MBOX_VF_ADD_DEL_MAC_REPLY:
enic_mbox_vf_handle_add_del_mac_reply(enic, payload,
payload_len, msg_num);
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH net-next v2 6/6] enic: configure V2 VF receive mode over mailbox
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
` (4 preceding siblings ...)
2026-09-29 18:58 ` [PATCH net-next v2 5/6] enic: manage V2 VF station and administrative MAC Satish Kharat
@ 2026-09-29 18:58 ` Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
5 siblings, 1 reply; 11+ messages in thread
From: Satish Kharat @ 2026-09-29 18:58 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Satish Kharat, sebaddel
V2 VFs cannot program receive filters directly. Add an asynchronous
receive-mode callback that sends individual unicast and multicast address
changes and packet-filter settings through the VF's one-request-at-a-time
mailbox path.
Complete all phases required for one address-list snapshot before returning
from the callback. Enable newly required broad receive coverage before
changing individual-address filters. Withdraw stale broad coverage before
installing its finite replacement. This preserves synchronous completion
when the netdev core executes the sleepable callback inline.
Use the netdev address-list synchronization state to batch operations:
sync_cnt == 0 identifies a new entry, refcount > sync_cnt identifies an
address that is still requested, and refcount == sync_cnt identifies an
installed entry which has become stale. The asynchronous netdev core
reconciles the resulting sync_cnt changes from the snapshot back to the
live list.
Track detailed per-address results so idempotent outcomes converge, stable
policy denials do not consume the retry budget, and failed deletes or
contradictory replies require a new VF registration. Keep the station
address out of both secondary address lists.
Deployed async PFs count the station address in the same 32-entry unicast
table used for secondary addresses. Request broad coverage at 32 secondary
unicast addresses so those PFs do not silently lose the final address.
Select the asynchronous netdev operations only for V2 VFs and detach the
device before mailbox teardown so no receive-mode callback can race
removal.
Assisted-by: LLM
Signed-off-by: Satish Kharat <satishkh@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic.h | 3 +
drivers/net/ethernet/cisco/enic/enic_main.c | 468 +++++++++++++++++++++++++++-
2 files changed, 458 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic.h b/drivers/net/ethernet/cisco/enic/enic.h
index 1e369e6704e1..80056b5f4124 100644
--- a/drivers/net/ethernet/cisco/enic/enic.h
+++ b/drivers/net/ethernet/cisco/enic/enic.h
@@ -387,6 +387,9 @@ struct enic {
*/
struct enic_mac_addr *mbox_reply_mac_addrs;
u16 mbox_reply_mac_count;
+ u16 vf_pkt_filter_requested;
+ u16 vf_pkt_filter_applied;
+ bool vf_pkt_filter_valid;
bool mbox_initialized;
/* PF: per-VF MBOX state, allocated when SRIOV V2 is enabled */
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 2132bfa9c8d3..382bac6669bf 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1051,6 +1051,7 @@ void enic_reset_addr_lists(struct enic *enic)
enic->mc_count = 0;
enic->uc_count = 0;
enic->flags = 0;
+ enic->vf_pkt_filter_valid = false;
}
static int enic_set_mac_addr(struct net_device *netdev, char *addr)
@@ -1352,8 +1353,12 @@ static void enic_vf_admin_mac_work(struct work_struct *work)
if (station_installed)
enic_vf_station_addr_set(enic, selected);
enic_vf_station_sync_reset(enic);
- if (enic->netdev->reg_state == NETREG_REGISTERED && changed)
- call_netdevice_notifiers(NETDEV_CHANGEADDR, enic->netdev);
+ if (enic->netdev->reg_state == NETREG_REGISTERED) {
+ if (changed)
+ call_netdevice_notifiers(NETDEV_CHANGEADDR,
+ enic->netdev);
+ netif_rx_mode_schedule_update(enic->netdev);
+ }
netdev_info(enic->netdev, "MBOX: admin MAC set to %pM\n", selected);
unlock:
@@ -1573,6 +1578,7 @@ static int enic_set_mac_address_dynamic(struct net_device *netdev, void *p)
enic_vf_station_addr_set(enic, addr);
enic_vf_station_sync_reset(enic);
enic_vf_admin_mac_cache_selected(enic, addr);
+ netif_rx_mode_schedule_update(netdev);
}
return err;
@@ -1615,17 +1621,380 @@ static int enic_set_mac_address(struct net_device *netdev, void *p)
return enic_dev_add_station_addr(enic);
}
+static u16 enic_rx_mode_to_pkt_filter(struct net_device *netdev,
+ unsigned int uc_count,
+ unsigned int mc_count)
+{
+ u16 flags = CMD_PFILTER_DIRECTED;
+
+ if (netdev->flags & IFF_MULTICAST)
+ flags |= CMD_PFILTER_MULTICAST;
+ if (netdev->flags & IFF_BROADCAST)
+ flags |= CMD_PFILTER_BROADCAST;
+ if ((netdev->flags & IFF_PROMISC) ||
+ uc_count > ENIC_UNICAST_PERFECT_FILTERS)
+ flags |= CMD_PFILTER_PROMISCUOUS;
+ if ((netdev->flags & IFF_ALLMULTI) ||
+ mc_count > ENIC_MULTICAST_PERFECT_FILTERS)
+ flags |= CMD_PFILTER_ALL_MULTICAST;
+
+ return flags;
+}
+
+/* Deployed async PFs count the station MAC in the 32-entry UC table. */
+static bool enic_vf_uc_needs_promisc(unsigned int uc_count)
+{
+ return uc_count >= ENIC_UNICAST_PERFECT_FILTERS;
+}
+
+static int enic_vf_set_pkt_filter(struct enic *enic, u16 flags,
+ u16 *applied_flags)
+{
+ u16 applied;
+ int err;
+
+ /* The PF can change trust policy independently and thereby withdraw
+ * broad modes. Always refresh those requests so the returned applied
+ * flags remain authoritative. The other modes are not trust-gated and
+ * can be reused for ordinary address-list churn.
+ */
+ if (!(flags & (CMD_PFILTER_PROMISCUOUS |
+ CMD_PFILTER_ALL_MULTICAST)) &&
+ enic->vf_pkt_filter_valid &&
+ enic->vf_pkt_filter_requested == flags) {
+ if (applied_flags)
+ *applied_flags = enic->vf_pkt_filter_applied;
+ return 0;
+ }
+
+ err = enic_mbox_vf_set_pkt_filter(enic,
+ !!(flags & CMD_PFILTER_DIRECTED),
+ !!(flags & CMD_PFILTER_MULTICAST),
+ !!(flags & CMD_PFILTER_BROADCAST),
+ !!(flags & CMD_PFILTER_PROMISCUOUS),
+ !!(flags & CMD_PFILTER_ALL_MULTICAST),
+ &applied);
+ if (err)
+ return err;
+
+ enic->vf_pkt_filter_requested = flags;
+ enic->vf_pkt_filter_applied = applied;
+ enic->vf_pkt_filter_valid = true;
+ if (applied_flags)
+ *applied_flags = applied;
+
+ return 0;
+}
+
+static void enic_vf_report_pkt_filter_denial(struct net_device *netdev,
+ u16 requested, u16 applied)
+{
+ if ((requested & CMD_PFILTER_PROMISCUOUS) &&
+ !(applied & CMD_PFILTER_PROMISCUOUS))
+ netdev_dbg(netdev, "PF policy denied promiscuous receive mode\n");
+ if ((requested & CMD_PFILTER_ALL_MULTICAST) &&
+ !(applied & CMD_PFILTER_ALL_MULTICAST))
+ netdev_dbg(netdev,
+ "PF policy denied all-multicast receive mode\n");
+}
+
+struct enic_vf_mac_op {
+ struct netdev_hw_addr *ha;
+ unsigned int *filter_count;
+};
+
+static unsigned int
+enic_vf_addr_list_count(const struct enic *enic,
+ const struct netdev_hw_addr_list *list)
+{
+ const struct netdev_hw_addr *ha;
+ unsigned int count = 0;
+
+ netdev_hw_addr_list_for_each(ha, list)
+ if (ha->refcount > ha->sync_cnt &&
+ !(enic->vf_station_addr_valid &&
+ ether_addr_equal(ha->addr, enic->vf_station_addr)))
+ count++;
+
+ return count;
+}
+
+static int enic_vf_collect_mac_ops(struct enic *enic,
+ struct netdev_hw_addr_list *list,
+ bool install_new,
+ unsigned int *filter_count,
+ struct enic_mac_addr *macs,
+ struct enic_vf_mac_op *ops,
+ u16 *num_ops)
+{
+ struct netdev_hw_addr *ha;
+ u16 pos = *num_ops;
+
+ /* Deletes precede adds so a full perfect-filter table has room for a
+ * replacement address in the same transaction.
+ */
+ netdev_hw_addr_list_for_each(ha, list) {
+ if (enic->vf_station_addr_valid &&
+ ether_addr_equal(ha->addr, enic->vf_station_addr))
+ continue;
+ if (!ha->sync_cnt ||
+ ha->refcount != ha->sync_cnt)
+ continue;
+ if (pos == ENIC_MBOX_MAX_MAC_OPS)
+ return -E2BIG;
+
+ ether_addr_copy(macs[pos].addr, ha->addr);
+ macs[pos].flags = 0;
+ ops[pos].ha = ha;
+ ops[pos].filter_count = filter_count;
+ pos++;
+ }
+
+ if (!install_new)
+ goto done;
+
+ netdev_hw_addr_list_for_each(ha, list) {
+ if (enic->vf_station_addr_valid &&
+ ether_addr_equal(ha->addr, enic->vf_station_addr))
+ continue;
+ if (ha->sync_cnt)
+ continue;
+ if (pos == ENIC_MBOX_MAX_MAC_OPS)
+ return -E2BIG;
+
+ ether_addr_copy(macs[pos].addr, ha->addr);
+ macs[pos].flags = cpu_to_le16(ENIC_MAC_ADDR_FLAG_ADD);
+ ops[pos].ha = ha;
+ ops[pos].filter_count = filter_count;
+ pos++;
+ }
+
+done:
+ *num_ops = pos;
+ return 0;
+}
+
+static int enic_vf_sync_mac_filters(struct enic *enic,
+ struct netdev_hw_addr_list *uc,
+ struct netdev_hw_addr_list *mc,
+ bool install_uc, bool install_mc)
+{
+ struct enic_vf_mac_op *ops;
+ struct enic_mac_addr *macs;
+ bool permanent_add = false;
+ bool reconnect = false;
+ bool retryable_add = false;
+ u16 num_ops = 0;
+ unsigned int i;
+ int err = 0;
+
+ /* A class can contribute at most its installed perfect filters as
+ * deletes and its finite perfect-filter limit as adds. Keep the wire
+ * batch large enough for both classes so an update always fits in one
+ * transaction and -E2BIG remains only a defensive state-corruption
+ * check.
+ */
+ BUILD_BUG_ON(ENIC_MBOX_MAX_MAC_OPS <
+ 2 * (ENIC_UNICAST_PERFECT_FILTERS +
+ ENIC_MULTICAST_PERFECT_FILTERS));
+
+ macs = kcalloc(ENIC_MBOX_MAX_MAC_OPS, sizeof(*macs), GFP_KERNEL);
+ if (!macs)
+ return -ENOMEM;
+ ops = kcalloc(ENIC_MBOX_MAX_MAC_OPS, sizeof(*ops), GFP_KERNEL);
+ if (!ops) {
+ err = -ENOMEM;
+ goto free_macs;
+ }
+
+ err = enic_vf_collect_mac_ops(enic, uc, install_uc,
+ &enic->uc_count,
+ macs, ops, &num_ops);
+ if (err)
+ goto free_ops;
+ err = enic_vf_collect_mac_ops(enic, mc, install_mc,
+ &enic->mc_count,
+ macs, ops, &num_ops);
+ if (err || !num_ops)
+ goto free_ops;
+
+ /* The protocol carries all changed unicast and multicast addresses in
+ * one request. Besides matching the native ABI, this bounds time spent
+ * in the RTNL-held asynchronous receive-mode callback to one MAC reply.
+ */
+ err = enic_mbox_vf_add_del_macs(enic, macs, num_ops);
+ if (err)
+ goto free_ops;
+
+ for (i = 0; i < num_ops; i++) {
+ u16 flags = le16_to_cpu(macs[i].flags);
+ bool add = flags & ENIC_MAC_ADDR_FLAG_ADD;
+
+ if (flags & ENIC_MAC_ADDR_FLAG_SKIPPED) {
+ if (add)
+ retryable_add = true;
+ else
+ reconnect = true;
+ continue;
+ }
+ if (flags & ENIC_MAC_ADDR_FLAG_PERMANENT_MASK) {
+ if (add)
+ permanent_add = true;
+ else
+ reconnect = true;
+ continue;
+ }
+
+ if (add) {
+ ops[i].ha->sync_cnt++;
+ ops[i].ha->refcount++;
+ (*ops[i].filter_count)++;
+ } else {
+ ops[i].ha->sync_cnt--;
+ ops[i].ha->refcount--;
+ if (WARN_ON_ONCE(!*ops[i].filter_count))
+ err = -EIO;
+ else
+ (*ops[i].filter_count)--;
+ }
+ }
+
+ /* A failed DELETE can leave hardware accepting an address no longer in
+ * the requested list. Registration is the fail-closed cleanup boundary.
+ * A skipped ADD changes no acceptance state and can use the bounded core
+ * retry path. Permanent ADD denials remain stable policy results.
+ */
+ if (reconnect) {
+ enic_mbox_vf_require_reconnect(enic);
+ err = -EIO;
+ } else if (retryable_add) {
+ err = -EAGAIN;
+ } else if (permanent_add) {
+ err = -EACCES;
+ }
+
+free_ops:
+ kfree(ops);
+free_macs:
+ kfree(macs);
+ return err;
+}
+
+static int enic_set_vf_rx_mode(struct net_device *netdev,
+ struct netdev_hw_addr_list *uc,
+ struct netdev_hw_addr_list *mc)
+{
+ struct enic *enic = netdev_priv(netdev);
+ unsigned int uc_count = enic_vf_addr_list_count(enic, uc);
+ unsigned int mc_count = enic_vf_addr_list_count(enic, mc);
+ u16 flags = enic_rx_mode_to_pkt_filter(netdev, uc_count, mc_count);
+ bool uc_overflow = enic_vf_uc_needs_promisc(uc_count);
+ bool mc_overflow = mc_count > ENIC_MULTICAST_PERFECT_FILTERS;
+ u16 broad_modes = CMD_PFILTER_PROMISCUOUS |
+ CMD_PFILTER_ALL_MULTICAST;
+ bool broad_enable_needed;
+ bool broad_withdrawal;
+ bool filter_needed;
+ bool target_filter_updated = false;
+ u16 prefilter_flags;
+ u16 applied_flags;
+ int err;
+
+ if (uc_overflow)
+ flags |= CMD_PFILTER_PROMISCUOUS;
+
+ if (!READ_ONCE(enic->vf_registered))
+ return -ENODEV;
+
+ filter_needed = !enic->vf_pkt_filter_valid ||
+ enic->vf_pkt_filter_requested != flags ||
+ (flags & broad_modes);
+ broad_enable_needed =
+ (!enic->vf_pkt_filter_valid && (flags & broad_modes)) ||
+ (flags & broad_modes & ~enic->vf_pkt_filter_requested);
+ broad_withdrawal = enic->vf_pkt_filter_valid &&
+ (enic->vf_pkt_filter_applied & broad_modes & ~flags);
+ if (broad_enable_needed) {
+ /* Establish newly required broad coverage before an independent
+ * exact-address rejection can block it. Retain any broad mode that
+ * is currently applied until its finite replacement is installed.
+ */
+ prefilter_flags = flags;
+ if (enic->vf_pkt_filter_valid)
+ prefilter_flags |= enic->vf_pkt_filter_applied & broad_modes;
+ err = enic_vf_set_pkt_filter(enic, prefilter_flags,
+ &applied_flags);
+ if (err)
+ return err;
+ target_filter_updated = prefilter_flags == flags;
+ enic_vf_report_pkt_filter_denial(netdev, flags, applied_flags);
+ }
+
+ if (broad_withdrawal) {
+ /* Withdraw stale broad acceptance before installing its finite
+ * replacement. Complete both phases in this callback so a synchronous
+ * receive-mode operation cannot return between them.
+ */
+ err = enic_vf_set_pkt_filter(enic, flags, &applied_flags);
+ if (err) {
+ /* The previously applied broad mode may still be active. Do
+ * not rely on the core's bounded retry budget to narrow it.
+ * Fresh registration is the fail-closed policy boundary. A
+ * local send timeout is already terminal and cannot use ordinary
+ * reconnect recovery.
+ */
+ if (!READ_ONCE(enic->mbox_tx_poisoned) &&
+ !READ_ONCE(enic->vf_mbox_reconnect_required))
+ enic_mbox_vf_require_reconnect(enic);
+ return err;
+ }
+ target_filter_updated = true;
+ enic_vf_report_pkt_filter_denial(netdev, flags, applied_flags);
+ }
+
+ /* Keep the finite subset of exact filters already installed for an
+ * overflowing class. Broad mode covers the remaining addresses when PF
+ * policy permits it, while the independent finite class can still make
+ * progress. When a class becomes finite again, stale broad acceptance was
+ * withdrawn above before this exact-filter reconciliation.
+ */
+ err = enic_vf_sync_mac_filters(enic, uc, mc, !uc_overflow,
+ !mc_overflow);
+ if (err == -EACCES) {
+ /* Do not spend the core retry budget repeating an exact operation
+ * that the PF rejected permanently. Finish any independent packet
+ * policy change below before returning success.
+ */
+ err = 0;
+ }
+ if (err)
+ return err;
+
+ if (!target_filter_updated && filter_needed) {
+ err = enic_vf_set_pkt_filter(enic, flags, &applied_flags);
+ if (err)
+ return err;
+ } else if (!target_filter_updated) {
+ applied_flags = enic->vf_pkt_filter_applied;
+ }
+
+ enic_vf_report_pkt_filter_denial(netdev, flags, applied_flags);
+
+ return 0;
+}
+
/* netif_tx_lock held, BHs disabled */
static void enic_set_rx_mode(struct net_device *netdev)
{
struct enic *enic = netdev_priv(netdev);
- int directed = 1;
- int multicast = (netdev->flags & IFF_MULTICAST) ? 1 : 0;
- int broadcast = (netdev->flags & IFF_BROADCAST) ? 1 : 0;
- int promisc = (netdev->flags & IFF_PROMISC) ||
- netdev_uc_count(netdev) > ENIC_UNICAST_PERFECT_FILTERS;
- int allmulti = (netdev->flags & IFF_ALLMULTI) ||
- netdev_mc_count(netdev) > ENIC_MULTICAST_PERFECT_FILTERS;
+ u16 filter_flags = enic_rx_mode_to_pkt_filter(netdev,
+ netdev_uc_count(netdev),
+ netdev_mc_count(netdev));
+ int directed = !!(filter_flags & CMD_PFILTER_DIRECTED);
+ int multicast = !!(filter_flags & CMD_PFILTER_MULTICAST);
+ int broadcast = !!(filter_flags & CMD_PFILTER_BROADCAST);
+ int promisc = !!(filter_flags & CMD_PFILTER_PROMISCUOUS);
+ int allmulti = !!(filter_flags & CMD_PFILTER_ALL_MULTICAST);
unsigned int flags = netdev->flags |
(allmulti ? IFF_ALLMULTI : 0) |
(promisc ? IFF_PROMISC : 0);
@@ -2233,6 +2602,10 @@ static int enic_open(struct net_device *netdev)
{
struct enic *enic = netdev_priv(netdev);
bool vf_mac_added = false;
+ u16 vf_filter_applied;
+ u16 vf_filter_flags;
+ unsigned int vf_mc_count;
+ unsigned int vf_uc_count;
unsigned int i;
int err, ret;
unsigned int max_pkt_len = netdev->mtu + VLAN_ETH_HLEN;
@@ -2313,7 +2686,6 @@ static int enic_open(struct net_device *netdev)
if (!enic_is_dynamic(enic) && !enic_is_sriov_vf(enic))
enic_dev_add_station_addr(enic);
-
if (enic_is_sriov_vf_v2(enic)) {
if (!READ_ONCE(enic->vf_registered)) {
netdev_err(netdev, "VF is not registered with its PF\n");
@@ -2331,9 +2703,38 @@ static int enic_open(struct net_device *netdev)
enic_vf_station_addr_set(enic, netdev->dev_addr);
enic_vf_station_sync_reset(enic);
vf_mac_added = true;
+
+ netif_addr_lock_bh(netdev);
+ vf_uc_count = enic_vf_addr_list_count(enic, &netdev->uc);
+ vf_mc_count = enic_vf_addr_list_count(enic, &netdev->mc);
+ netif_addr_unlock_bh(netdev);
+
+ vf_filter_flags = enic_rx_mode_to_pkt_filter(netdev,
+ vf_uc_count,
+ vf_mc_count);
+ if (enic_vf_uc_needs_promisc(vf_uc_count))
+ vf_filter_flags |= CMD_PFILTER_PROMISCUOUS;
+ err = enic_vf_set_pkt_filter(enic, vf_filter_flags,
+ &vf_filter_applied);
+ if (err) {
+ netdev_err(netdev,
+ "Failed to configure VF packet filter: %d\n",
+ err);
+ goto err_out_disable_wq;
+ }
+ if ((enic_vf_uc_needs_promisc(vf_uc_count) &&
+ !(vf_filter_applied & CMD_PFILTER_PROMISCUOUS)) ||
+ (vf_mc_count > ENIC_MULTICAST_PERFECT_FILTERS &&
+ !(vf_filter_applied & CMD_PFILTER_ALL_MULTICAST))) {
+ netdev_err(netdev,
+ "PF denied receive mode required by VF address lists\n");
+ err = -EACCES;
+ goto err_out_disable_wq;
+ }
}
- enic_set_rx_mode(netdev);
+ if (!enic_is_sriov_vf_v2(enic))
+ enic_set_rx_mode(netdev);
netif_tx_wake_all_queues(netdev);
@@ -2971,6 +3372,13 @@ static void enic_reset(struct work_struct *work)
if (err)
netdev_err(enic->netdev,
"Failed to reopen datapath after reset: %d\n", err);
+ else if (enic_is_sriov_vf_v2(enic)) {
+ /* Internal reset bypasses __dev_open(), which normally schedules the
+ * asynchronous receive-mode upload after ndo_open. Schedule an update
+ * after enic_reset_addr_lists() marked the lists unsynchronized.
+ */
+ netif_rx_mode_schedule_update(enic->netdev);
+ }
/* A PF reopens its admin channel after the datapath and re-pushes link
* state. The VF handshake, which open depends on, completed above.
@@ -3038,6 +3446,8 @@ static void enic_tx_hang_reset(struct work_struct *work)
if (err)
netdev_err(enic->netdev,
"Failed to reopen datapath after hang reset: %d\n", err);
+ else if (enic_is_sriov_vf_v2(enic))
+ netif_rx_mode_schedule_update(enic->netdev);
/* A PF reopens its admin channel after the datapath and re-pushes link
* state. The VF handshake, which open depends on, completed above.
@@ -3292,6 +3702,30 @@ static const struct net_device_ops enic_netdev_dynamic_ops = {
.ndo_features_check = enic_features_check,
};
+static const struct net_device_ops enic_netdev_vf_v2_ops = {
+ .ndo_open = enic_open,
+ .ndo_stop = enic_stop,
+ .ndo_start_xmit = enic_hard_start_xmit,
+ .ndo_get_stats64 = enic_get_stats,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_rx_mode_async = enic_set_vf_rx_mode,
+ .ndo_set_mac_address = enic_set_mac_address_dynamic,
+ .ndo_change_mtu = enic_change_mtu,
+ .ndo_vlan_rx_add_vid = enic_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = enic_vlan_rx_kill_vid,
+ .ndo_tx_timeout = enic_tx_timeout,
+ .ndo_set_vf_port = enic_set_vf_port,
+ .ndo_get_vf_port = enic_get_vf_port,
+ .ndo_set_vf_mac = enic_set_vf_mac,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = enic_poll_controller,
+#endif
+#ifdef CONFIG_RFS_ACCEL
+ .ndo_rx_flow_steer = enic_rx_flow_steer,
+#endif
+ .ndo_features_check = enic_features_check,
+};
+
static const struct net_device_ops enic_netdev_ops = {
.ndo_open = enic_open,
.ndo_stop = enic_stop,
@@ -3959,7 +4393,9 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
enic->rx_coalesce_usecs = enic->tx_coalesce_usecs;
- if (enic_is_dynamic(enic) || enic_is_sriov_vf(enic))
+ if (enic_is_sriov_vf_v2(enic))
+ netdev->netdev_ops = &enic_netdev_vf_v2_ops;
+ else if (enic_is_dynamic(enic) || enic_is_sriov_vf(enic))
netdev->netdev_ops = &enic_netdev_dynamic_ops;
else
netdev->netdev_ops = &enic_netdev_ops;
@@ -4106,12 +4542,18 @@ static void enic_remove(struct pci_dev *pdev)
/* Close the admin channel and unregister from the PF before
* unregister_netdev() to prevent a late PF notification from
- * touching a netdev that is being torn down. VF_UNREGISTER is the
+ * touching a netdev that is being torn down. VF_UNREGISTER is the
* protocol teardown operation: the PF removes all VF-requested
* configuration, including the station address, before replying.
+ * Detach first while holding RTNL so any running asynchronous
+ * receive-mode update has completed and queued updates cannot enter
+ * the mailbox callback.
*/
if (enic_is_sriov_vf_v2(enic)) {
enic_vf_admin_mac_quiesce(enic);
+ rtnl_lock();
+ netif_device_detach(netdev);
+ rtnl_unlock();
if (READ_ONCE(enic->vf_registered)) {
int unreg_err = enic_mbox_vf_unregister(enic);
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown
2026-09-29 18:58 ` [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown Satish Kharat
@ 2026-10-03 19:50 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-03 19:50 UTC (permalink / raw)
To: satishkh
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, netdev,
linux-kernel, sebaddel
Thank you for your contribution! Sashiko AI review found 11 potential
issue(s) to consider.
Critical: 0 · High: 2 · Medium: 6 · Low: 3
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] enic_reset() now has two new error exits, taken when
enic_dev_soft_reset() fails or when the newly added vnic_dev_init()…
- [High] The patch adds vf_datapath_open so that "a failed internal reset
cannot stop it twice".
- [Medium] The commit message says the timed-out DMA mapping is left "for
admin-channel teardown to reclaim safely".
- [Medium] vf_mbox_recovery_active is a plain bool.
- [Medium] vf_mbox_recovery_active stays true for the whole run of the
reset worker, including the time after enic_open() has committed…
- [Medium] The patch makes several admin-RQ drop paths trigger full VF
recovery (RX quarantine, carrier off, sends disabled, a soft reset…
- [Medium] The new V2 VF error exit in enic_open() (`goto
err_out_dev_disable`) runs after enic_dev_enable() has succeeded, when…
- [Medium] The new vnic_dev_init() call in enic_reset() issues CMD_INIT,
or on older firmware CMD_INIT_v1 + CMD_GET_MAC_ADDR + CMD_ADDR_ADD,…
- [Low] Concern: the patch has no Fixes: tag even though it fixes V2
mailbox behaviour introduced by commit 1f0c856b596337.
- [Low] Concern: the commit message and comments assume a VF MAC/receive-
filter mailbox protocol and a station/filter replay that do not exist…
- [Low] Concern: the new enic_reset_addr_lists() calls in enic_open() and
enic_admin_chan_reopen() run __dev_uc_unsync()/__dev_mc_unsync()…
--
enic: recover V2 VF mailbox when PF state is unknown
This patch adds recovery for the V2 VF mailbox after a lost or malformed
reply. VF RX is quarantined until the admin channel has been rebuilt,
re-registered and replayed at an open/reset boundary, and the channel is
poisoned for good after a send-completion timeout. It also defers VF
notification ACKs to a work item, moves the VF handshake ahead of
enic_open() in the reset workers, calls vnic_dev_init() after soft reset
for non-dynamic vNICs, and tracks whether the V2 datapath is open.
> Assisted-by: LLM
> Signed-off-by: Satish Kharat <satishkh@cisco.com>
[Severity: Low]
This isn't a bug, but there's no Fixes: tag, even though the patch changes
V2 mailbox behaviour from commit 1f0c856b596337. The series reads as a
continuation of the V2 mailbox bring-up with the PF side still dormant, so
leaving it out may be fine for net-next. Was that intentional?
> diff --git a/drivers/net/ethernet/cisco/enic/enic_admin.c b/drivers/net/ethernet/cisco/enic/enic_admin.c
> index 61c82b48044d0..30c0a5c89a2d2 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_admin.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_admin.c
[ ... ]
> @@ -194,8 +203,10 @@ unsigned int enic_admin_rq_cq_service(struct enic *enic)
> rq_desc = desc;
> bwf = le16_to_cpu(rq_desc->bytes_written_flags);
> bytes_written = bwf & CQ_ENET_RQ_DESC_BYTES_WRITTEN_MASK;
> - if (bytes_written > buf->len)
> + if (bytes_written > buf->len) {
> + enic_admin_rx_lost(enic);
> goto next_desc;
> + }
[Severity: Medium]
Should the sender be checked before these drops trigger a full VF recovery?
This path, and the TRUNCATED and missing FCS_OK paths just below it, call
enic_admin_rx_lost() before rq_desc->vlan (the hardware-verified sender) is
read.
The same thing happens in two other places:
- Backlog overflow in enic_admin_msg_enqueue(). Every message counts
toward ENIC_ADMIN_MSG_MAX, even ones that are later dropped as non-PF.
- The short-message case in enic_mbox_recv_handler(), which calls
enic_mbox_vf_require_reconnect() before the
src_vnic_id == ENIC_MBOX_DST_PF check.
The driver already treats non-PF senders on the VF admin RQ as spoofed. If
the adapter delivers VF-to-VF admin messages, could another VF force this
VF through repeated resets?
One such message during re-registration also makes
enic_admin_chan_reopen() return -EAGAIN. Since self-requeue is suppressed,
the VF then stays quarantined.
I couldn't confirm that firmware routes this kind of traffic. The later
patches in the series don't add sender checks to these paths either.
[ ... ]
> diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
> index 9086e6dd558aa..28738bd14fa7c 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
[ ... ]
> @@ -1736,6 +1740,30 @@ static int enic_open(struct net_device *netdev)
[ ... ]
> + if (enic_is_sriov_vf_v2(enic) &&
> + (!enic->admin_chan_up || !READ_ONCE(enic->vf_registered) ||
> + READ_ONCE(enic->vf_mbox_reconnect_required))) {
> + /* Re-registration makes the PF discard the old VF-requested
> + * filters. Clear the netdev-core synchronization state so the
> + * receive-mode callback replays the current address lists.
> + */
[Severity: Low]
This isn't a bug, but this comment and the commit message ("restore receive
traffic after the station address and filters have been replayed") describe
a VF MAC/receive-filter mailbox protocol and a station/filter replay that
this patch doesn't contain yet.
They arrive later in the series:
- "enic: validate V2 VF configuration replies"
- "enic: manage V2 VF station and administrative MAC"
- "enic: configure V2 VF receive mode over mailbox"
Could the commit message say that it depends on those follow-ups?
> + enic_reset_addr_lists(enic);
[Severity: Low]
Does enic_reset_addr_lists() need netif_addr_lock_bh() here?
It calls __dev_uc_unsync() and __dev_mc_unsync() without the address lock.
Writers such as IPv6 MLD can change dev->mc under the address lock without
holding RTNL. The new call in enic_admin_chan_reopen() has the same issue.
This appears to be fixed later in the series, where enic_reset_addr_lists()
wraps both calls in netif_addr_lock_bh().
> + if (enic->admin_chan_up)
> + enic_admin_channel_close(enic);
> + err = enic_admin_chan_reopen(enic);
> + if (err)
> + return err;
> + }
[ ... ]
> @@ -1794,17 +1822,41 @@ static int enic_open(struct net_device *netdev)
> netdev_err(netdev, "Failed to enable device: %d\n", err);
> goto err_out_dev_enable;
> }
> + if (enic_is_sriov_vf_v2(enic)) {
[ ... ]
> + spin_unlock_bh(&enic->mbox_state_lock);
> + if (err) {
> + netdev_err(netdev,
> + "MBOX state changed during VF datapath open\n");
> + goto err_out_dev_disable;
> + }
> + }
[ ... ]
> +err_out_dev_disable:
> + enic_dev_disable(enic);
> err_out_dev_enable:
> for (i = 0; i < enic->rq_count; i++)
> napi_disable(&enic->napi[i]);
[Severity: Medium]
Is this unwind enough once enic_dev_enable() has succeeded?
The existing error labels were written for failures before the device was
enabled. By this point the RQs are enabled and filled, the WQs are enabled,
and the adapter may already have written CQEs.
The unwind disables the device and queues and calls vnic_rq_clean(). It
doesn't do the vnic_cq_clean(), vnic_intr_clean() and vnic_wq_clean() calls
that __enic_stop() does after a live enable.
A later administrative enic_open() doesn't call
enic_init_vnic_resources(). On that open, could NAPI process stale CQEs
against the repositioned RQ ring? That could deliver buffers the device
never wrote, or reach an unposted buffer whose os_buf is NULL.
> @@ -1834,12 +1886,20 @@ static int enic_open(struct net_device *netdev)
> }
>
> /* rtnl lock is held, process context */
> -static int enic_stop(struct net_device *netdev)
> +static int __enic_stop(struct net_device *netdev, bool remove_vf_station)
[ ... ]
> + if (enic_is_sriov_vf_v2(enic) && !enic->vf_datapath_open)
> + return 0;
> + (void)remove_vf_station;
[ ... ]
> @@ -1893,10 +1953,17 @@ static int enic_stop(struct net_device *netdev)
> vnic_cq_clean(&enic->cq[i]);
> for (i = 0; i < enic->intr_count; i++)
> vnic_intr_clean(&enic->intr[i]);
> + if (enic_is_sriov_vf_v2(enic))
> + enic->vf_datapath_open = false;
[Severity: High]
Does this cover the early return paths in __enic_stop()?
The flag is only cleared here, on the last line. Earlier in the function,
after napi_disable() has already run, there is:
for (i = 0; i < enic->wq_count; i++) {
err = vnic_wq_disable(&enic->wq[i].vwq);
if (err)
return err;
}
The vnic_rq_disable() loop has the same shape. Both returns skip
enic_free_intr() and leave vf_datapath_open set to true.
enic_reset() and enic_tx_hang_reset() ignore the return value of
__enic_stop() and go on to enic_open(). There, enic_request_intr() fails
because the MSI-X vectors are still requested. enic_open() returns an error
and vf_datapath_open is still true.
A later ifdown then gets past the guard and calls napi_disable() again on
NAPI instances that are already disabled. That spins forever under RTNL.
A hung WQ in enic_tx_hang_reset() seems a likely way for vnic_wq_disable()
to time out. The commit message says the tracking exists "so a failed
internal reset cannot stop it twice". Is this path meant to be covered too?
[ ... ]
> @@ -2253,79 +2346,128 @@ static void enic_admin_chan_reopen(struct enic *enic)
[ ... ]
> static void enic_reset(struct work_struct *work)
> {
> struct enic *enic = container_of(work, struct enic, reset);
> + bool vf_recovery = enic_is_sriov_vf_v2(enic);
> + int err;
>
> if (!netif_running(enic->netdev))
> return;
> + if (vf_recovery)
> + WRITE_ONCE(enic->vf_mbox_recovery_active, true);
>
> rtnl_lock();
[Severity: Medium]
Can vf_mbox_recovery_active actually suppress self-requeue when
enic_reset() and enic_tx_hang_reset() are both queued?
It's a plain bool. Both workers set it to true before rtnl_lock() and to
false before rtnl_unlock().
If both work items are queued, the first one to finish clears the flag
while the second is still blocked on RTNL. The second worker then runs
enic_admin_chan_reopen() with the flag false. Any mailbox fault during that
handshake reaches:
enic_mbox_vf_kick_recovery()
if (netif_running(enic->netdev) &&
!READ_ONCE(enic->vf_mbox_recovery_active))
schedule_work(&enic->reset);
That requeues the reset, which the commit message says shouldn't happen.
There's a second gap. The check-then-schedule in
enic_mbox_vf_kick_recovery() runs without a lock, and the worker only sets
the flag after it has been dequeued. A fault in between can also requeue.
Would a lock-protected state, or a counter tied to the reset work, hold up
better here than a flag that is set on entry and cleared on exit?
[ ... ]
> + __enic_stop(enic->netdev, false);
> if (enic_has_admin_chan(enic))
> enic_admin_channel_close(enic);
>
> - enic_stop(enic->netdev);
> if (enic_is_sriov_vf_v2(enic))
> enic_mbox_vf_link_state_reset(enic);
> + err = enic_dev_soft_reset(enic);
> + if (err)
> + goto reset_out;
[Severity: High]
Can these new error exits leave non-V2-VF devices with NAPI disabled while
netif_running() is still true?
By this point __enic_stop(enic->netdev, false) has disabled NAPI, freed the
IRQs and destroyed the page pools. If enic_dev_soft_reset() or
vnic_dev_init() fails, goto reset_out skips enic_open() for every ENIC
device type.
The double-stop guard added to __enic_stop() only covers V2 VFs:
if (enic_is_sriov_vf_v2(enic) && !enic->vf_datapath_open)
return 0;
On a PF, a legacy VF or a dynamic vNIC, a later ifdown or unregister calls
enic_stop()->__enic_stop()->napi_disable() on NAPI instances that are
already disabled. napi_disable_locked() then loops forever, holding RTNL,
waiting for NAPIF_STATE_SCHED / NAPIF_STATE_NPSVC to clear.
Before this patch, enic_reset() ignored the result of enic_dev_soft_reset()
and always called enic_open(). The reset is scheduled from
enic_isr_msix_err() and enic_isr_legacy() on WQ/RQ errors. That is when a
devcmd failure or an enic_dev_wait() timeout is most likely.
> +
> + if (!enic_is_dynamic(enic)) {
> + err = vnic_dev_init(enic->vdev, 0);
[Severity: Medium]
Should this vnic_dev_init() call hold enic->devcmd_lock?
vnic_dev_init() issues CMD_INIT through the shared devcmd registers (on
older firmware, CMD_INIT_v1 + CMD_GET_MAC_ADDR + CMD_ADDR_ADD). At this
point the netdev is registered and running.
Other runtime devcmd users take the lock. That includes one reachable from
ndo_get_stats64 without RTNL, for example by reading /proc/net/dev:
enic_dev_stats_dump()
spin_lock_bh(&enic->devcmd_lock);
err = vnic_dev_stats_dump(enic->vdev, vstats);
If the two interleave, can one command's arguments overwrite the other's in
the devcmd registers? The enic_dev_soft_reset() call just above was already
unlocked, but this adds up to three more unserialized devcmds.
> + if (err) {
> + netdev_err(enic->netdev,
> + "vNIC init after soft reset failed: %d\n",
> + err);
> + goto reset_out;
> + }
> + }
[ ... ]
> +reset_out:
> /* Allow infiniband to fiddle with the device again */
> enic_set_api_busy(enic, false);
>
> call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev);
>
> +unlock:
> + if (vf_recovery)
> + WRITE_ONCE(enic->vf_mbox_recovery_active, false);
> rtnl_unlock();
> }
[Severity: Medium]
What happens to a mailbox fault that arrives after enic_open() has
committed but before this flag is cleared?
Once enic_open() has cleared vf_rx_quarantined and turned carrier on, the
worker still runs enic_set_api_busy(false) and
call_netdevice_notifiers(NETDEV_REBOOT) with vf_mbox_recovery_active set.
Take an admin RX fault in that window:
enic_admin_rq_cq_service()
enic_admin_rx_lost()
enic_mbox_vf_require_reconnect()
enic_mbox_vf_kick_recovery()
This quarantines RX, disables sends and turns carrier off. It then skips
schedule_work() because the flag is still true.
The worker clears the flag here and returns without checking for a fault
after its commit point, so no reset gets scheduled. Wouldn't the VF then
drop every received frame, with carrier off, until someone runs down/up by
hand?
The commit message describes that outcome for a failed recovery handshake,
not a successful one.
[ ... ]
> diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
> index b8a18d9682b2f..30a5f6fda676d 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
> @@ -149,7 +149,20 @@ static int enic_mbox_send_msg_id(struct enic *enic, u8 msg_type,
> * or free the buffer: the device may still DMA from dma_addr.
> * Mark the channel unusable so no further sends are attempted.
> */
> + spin_lock_bh(&enic->mbox_state_lock);
> WRITE_ONCE(enic->mbox_send_disabled, true);
> + WRITE_ONCE(enic->mbox_tx_poisoned, true);
[Severity: Medium]
Is the admin-channel teardown actually safe for this buffer?
The commit message says the send-timeout path leaves "the timed-out DMA
mapping for admin-channel teardown to reclaim safely". But
enic_admin_channel_close() only warns when the queue disable fails, and
then carries on:
err = vnic_wq_disable(&enic->admin_wq);
if (err)
netdev_warn(enic->netdev,
"Failed to disable admin WQ: %d\n", err);
...
vnic_wq_clean(&enic->admin_wq, enic_admin_wq_buf_clean);
...
enic_admin_free_resources(enic);
enic_admin_wq_buf_clean() calls dma_unmap_single() and kfree() on the
buffer that this path deliberately kept alive. enic_admin_free_resources()
then frees the ring.
In enic_reset() and enic_tx_hang_reset(), this close also runs before
enic_dev_soft_reset() and enic_dev_hang_reset(), which are the operations
that would fence the admin WQ.
Suppose vnic_wq_disable() also times out on a queue that already missed its
completion. Can the device then still fetch the freed descriptor ring, or
read a buffer that has been freed and reused?
The new comment in enic.h ("until reset/DMA fencing is proven") seems to
concede that teardown doesn't prove the device has stopped.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-b4-enic-sriov-v2-vf-receive-control-v2-0-5962c10061b2%40cisco.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 4/6] enic: validate V2 VF configuration replies
2026-09-29 18:58 ` [PATCH net-next v2 4/6] enic: validate V2 VF configuration replies Satish Kharat
@ 2026-10-03 19:50 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-03 19:50 UTC (permalink / raw)
To: satishkh
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, netdev,
linux-kernel, sebaddel
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] At this commit the message says the patch adds VF-side
administrative-MAC notification operations, but…
- [Low] enic_mbox_vf_add_del_macs(), enic_mbox_vf_add_del_mac() and
enic_mbox_vf_set_pkt_filter() have no callers at this commit, and…
--
enic: validate V2 VF configuration replies
This commit adds VF-side V2 mailbox message types for MAC filter add/delete
and packet-filter settings, and validates their replies (framing, echoed
entries, result counts and applied flags). A reply that is malformed or
contradictory, or that reports registration loss, forces the VF to
re-register. The PF side stays dormant.
> Add the established V2 mailbox operations for MAC filters,
> administrative-MAC notifications, and packet-filter settings. Keep request
> buffers alive until the VF request completes so detailed replies can be
> checked entry by entry.
[ ... ]
> diff --git a/drivers/net/ethernet/cisco/enic/enic_mbox.c b/drivers/net/ethernet/cisco/enic/enic_mbox.c
> index 30a5f6fda676d..265b58cd39be3 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_mbox.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_mbox.c
[ ... ]
> @@ -680,6 +905,34 @@ static bool enic_mbox_vf_payload_ok(struct enic *enic, u8 msg_type,
> return true;
> }
>
> +static void enic_mbox_vf_malformed_msg(struct enic *enic, u8 msg_type,
> + u64 msg_num)
> +{
> + u8 expected;
> +
> + switch (msg_type) {
> + case ENIC_MBOX_PF_LINK_STATE_NOTIF:
> + case ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF:
> + enic_mbox_vf_require_reconnect(enic);
> + return;
[ ... ]
> @@ -718,11 +977,21 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
> size_t exp = sizeof(struct enic_mbox_pf_link_state_notif_msg);
>
> if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
> - payload_len, exp))
> + payload_len, exp)) {
> + enic_mbox_vf_malformed_msg(enic, hdr->msg_type, msg_num);
> return;
> + }
> enic_mbox_vf_handle_link_state(enic, payload, msg_num);
> break;
> }
> + case ENIC_MBOX_VF_ADD_DEL_MAC_REPLY:
> + enic_mbox_vf_handle_add_del_mac_reply(enic, payload,
> + payload_len, msg_num);
> + break;
> + case ENIC_MBOX_VF_SET_PKT_FILTER_REPLY:
> + enic_mbox_vf_handle_set_pkt_filter_reply(enic, payload,
> + payload_len, msg_num);
> + break;
> default:
> netdev_dbg(enic->netdev,
> "MBOX: VF unhandled msg type %u\n",
[Severity: Low]
Is the administrative-MAC part of the commit message accurate for this
commit? The commit message says:
Add the established V2 mailbox operations for MAC filters,
administrative-MAC notifications, and packet-filter settings.
This patch adds cases to enic_mbox_vf_process_msg() for
ENIC_MBOX_VF_ADD_DEL_MAC_REPLY and ENIC_MBOX_VF_SET_PKT_FILTER_REPLY. There
is no case for ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF, so:
- A well-formed notification falls through to the default netdev_dbg()
branch. It never gets an ENIC_MBOX_PF_SET_ADMIN_MAC_ACK.
- A malformed notification goes through enic_mbox_vf_malformed_msg(),
which forces a reconnect.
The later patch in this series, "enic: manage V2 VF station and
administrative MAC", appears to add the ENIC_MBOX_PF_SET_ADMIN_MAC_NOTIF
case and enic_mbox_vf_handle_admin_mac(). Also, no in-tree PF sends this
notification yet, because the V2 PF path is dormant.
Could the commit message say that this patch only adds the admin-MAC
message definitions, and that the later patch adds the handling?
[ ... ]
> @@ -880,14 +1166,182 @@ int enic_mbox_vf_unregister(struct enic *enic)
> if (registered)
> return -EACCES;
> return 0;
> }
>
> +int enic_mbox_vf_add_del_macs(struct enic *enic,
> + struct enic_mac_addr *macs, u16 num_macs)
> +{
[ ... ]
> +int enic_mbox_vf_add_del_mac(struct enic *enic, const u8 *addr, bool add,
> + bool station)
> +{
[ ... ]
> +int enic_mbox_vf_set_pkt_filter(struct enic *enic, int directed,
> + int multicast, int broadcast,
> + int promisc, int allmulti, u16 *applied_flags)
> +{
[Severity: Low]
This isn't a bug, but at this point in the series nothing calls
enic_mbox_vf_add_del_macs(), enic_mbox_vf_add_del_mac() or
enic_mbox_vf_set_pkt_filter(). ENIC_MAC_ADDR_FLAG_PERMANENT_MASK in
enic_mbox.h is also unused.
Two later patches wire them up: "enic: manage V2 VF station and
administrative MAC" and "enic: configure V2 VF receive mode over mailbox".
The callers are enic_vf_station_addr_del(), enic_vf_station_addr_replace(),
enic_vf_sync_mac_filters() and enic_vf_set_pkt_filter().
Would it make sense to move these helpers into the patches that add their
callers?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-b4-enic-sriov-v2-vf-receive-control-v2-0-5962c10061b2%40cisco.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 5/6] enic: manage V2 VF station and administrative MAC
2026-09-29 18:58 ` [PATCH net-next v2 5/6] enic: manage V2 VF station and administrative MAC Satish Kharat
@ 2026-10-03 19:50 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-03 19:50 UTC (permalink / raw)
To: satishkh
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, netdev,
linux-kernel, sebaddel
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 3 (1)
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] At this commit, enic_remove() runs
enic_vf_admin_mac_quiesce(), VF_UNREGISTER and…
- [Low] At this commit, enic_vf_station_sync_reset() drops the core sync
reference for the uc entry that matches vf_station_addr only once.
- [Low] The comment in enic_remove() says "VF_UNREGISTER is the protocol
teardown operation: the PF removes all VF-requested configuration,…
- [Low] At this commit, a runtime station change does not reprogram an
old station address A that is still wanted as a secondary unicast…
Pre-existing issues:
- [Low] enic_reset_addr_lists() now holds netif_addr_lock_bh() only
around __dev_uc_unsync()/__dev_mc_unsync().
--
enic: manage V2 VF station and administrative MAC
V2 VFs now manage the station MAC through a mailbox DEL+ADD replace
request. The address is installed on open and removed on an ordinary
stop. A delayed worker applies PF administrative MAC notifications, the
admin MAC policy is re-read after every VF registration, and the worker
is quiesced and rearmed around teardown and reset.
> diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
> index 28738bd14fa7c..2132bfa9c8d38 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
[ ... ]
> @@ -1040,8 +1043,10 @@ void enic_reset_addr_lists(struct enic *enic)
> {
> struct net_device *netdev = enic->netdev;
>
> + netif_addr_lock_bh(netdev);
> __dev_uc_unsync(netdev, NULL);
> __dev_mc_unsync(netdev, NULL);
> + netif_addr_unlock_bh(netdev);
>
> enic->mc_count = 0;
> enic->uc_count = 0;
[Severity: Low]
This is a pre-existing issue, and this patch did not introduce it. The
baseline did both the unsync and the counter reset with no lock at all,
so this change only narrows the window.
The addr lock now covers only the two unsync calls. mc_count, uc_count
and flags are still reset after netif_addr_unlock_bh().
Devices that use the legacy ndo_set_rx_mode path update these counters
under addr_list_lock only:
enic_set_rx_mode()
__dev_uc_sync(netdev, enic_uc_sync, enic_uc_unsync);
enic_uc_sync()
enic->uc_count++;
Can an rx_mode run between the unlock and the counter reset leave
uc_count or mc_count out of step with the core sync state? If uc_count
underflows, the ENIC_*_PERFECT_FILTERS limit check would no longer work.
Could the counter resets move inside the locked section?
For V2 VFs, the last patch in the series ("enic: configure V2 VF receive
mode over mailbox") switches to ndo_set_rx_mode_async.
netif_rx_mode_run() is serialized by RTNL, and so are the callers of
enic_reset_addr_lists().
[ ... ]
> @@ -1072,6 +1532,52 @@ static int enic_set_mac_address_dynamic(struct net_device *netdev, void *p)
[ ... ]
> + /* Keep the old software address visible until the complete station
> + * replacement proves convergence.
> + */
> + err = enic_vf_station_addr_replace(enic, addr);
> + if (err)
> + return err;
> +
> + enic_vf_station_sync_reset(enic);
> + err = enic_set_mac_addr(netdev, addr);
> + if (!err) {
> + enic_vf_station_addr_set(enic, addr);
> + enic_vf_station_sync_reset(enic);
> + enic_vf_admin_mac_cache_selected(enic, addr);
> + }
> +
> + return err;
> + }
[Severity: Low]
At this commit, if the old station address A is also in the uc list as a
secondary address, does anything reprogram A after the station moves?
enic_vf_station_addr_replace() deletes A at the PF, and
enic_vf_station_sync_reset() unsyncs A's core entry. No rx_mode update
is scheduled afterwards, so A would not be added back as a secondary
filter.
The nonzero-policy path in enic_vf_admin_mac_work() has a similar gap:
enic_vf_admin_mac_work() {
...
} else if (enic->vf_station_addr_valid &&
!ether_addr_equal(enic->vf_station_addr, selected)) {
mutated = true;
err = enic_vf_station_addr_del(enic);
...
changed = !ether_addr_equal(enic->netdev->dev_addr, selected);
enic_vf_station_sync_reset(enic);
...
}
enic_vf_station_addr_del() clears vf_station_addr_valid on success. The
first enic_vf_station_sync_reset() then returns early, and a re-synced
entry for A can keep sync_cnt=1 while the PF no longer has a filter for
A.
Could receive for A be lost until the next rx_mode event?
The last patch in the series, "enic: configure V2 VF receive mode over
mailbox", fixes this:
- It adds netif_rx_mode_schedule_update() here and after the admin-MAC
update in enic_vf_admin_mac_work().
- enic_vf_collect_mac_ops() skips the station address, so A has
sync_cnt==0 while it is the station.
Would it make sense to schedule the rx_mode update in this patch too, so
that intermediate commits don't have this behaviour?
[ ... ]
> @@ -1807,6 +2314,25 @@ static int enic_open(struct net_device *netdev)
> if (!enic_is_dynamic(enic) && !enic_is_sriov_vf(enic))
> enic_dev_add_station_addr(enic);
>
> + if (enic_is_sriov_vf_v2(enic)) {
[ ... ]
> + enic_vf_station_addr_set(enic, netdev->dev_addr);
> + enic_vf_station_sync_reset(enic);
> + vf_mac_added = true;
> + }
> +
> enic_set_rx_mode(netdev);
[Severity: Low]
enic_vf_station_sync_reset() drops the core sync reference for the
station entry only once. At this commit V2 VFs still use
enic_set_rx_mode(), which runs on the very next line, and enic_uc_sync()
does not exclude vf_station_addr:
enic_uc_sync() {
...
enic_dev_add_addr(enic, uc_addr);
enic->uc_count++;
...
}
Won't this re-sync the station entry right away?
If that uc entry is removed later, enic_uc_unsync() would call
enic_dev_del_addr() for the station address. It is not clear whether
that devcmd reaches the PF's address-keyed ledger for V2 VFs. The last
patch says V2 VFs cannot program receive filters directly.
The last patch in the series, "enic: configure V2 VF receive mode over
mailbox", resolves this:
- V2 VFs move to enic_set_vf_rx_mode() via ndo_set_rx_mode_async.
- enic_vf_collect_mac_ops() and enic_vf_addr_list_count() skip
vf_station_addr.
- enic_open() calls enic_set_rx_mode() only for non-V2 devices.
[ ... ]
> @@ -3542,9 +4106,13 @@ static void enic_remove(struct pci_dev *pdev)
>
> /* Close the admin channel and unregister from the PF before
> * unregister_netdev() to prevent a late PF notification from
> - * touching a netdev that is being torn down.
> + * touching a netdev that is being torn down. VF_UNREGISTER is the
> + * protocol teardown operation: the PF removes all VF-requested
> + * configuration, including the station address, before replying.
> */
[Severity: Low]
The VF now relies on this contract to skip the station DEL_MAC on
remove and on internal reset (__enic_stop(netdev, false)).
The in-tree PF side doesn't do this yet. enic_mbox_pf_handle_unregister()
only does:
enic->vf_state[vf_id].registered = false;
and replies. enic_mbox_pf_process_msg() has no case for
ENIC_MBOX_VF_ADD_DEL_MAC_REQUEST or ENIC_MBOX_PF_SET_ADMIN_MAC_ACK, so
both fall through to -EOPNOTSUPP.
This can't be reached today. enic_driver has no .sriov_configure, and
the comment above enic_sriov_configure() says the V2 helpers are
"defined but not yet wired". The earlier patch "enic: validate V2 VF
configuration replies" also says PF-side MAC handling must be added
before that callback is wired.
Should this comment say it describes the PF protocol contract rather
than the in-tree PF? That would remind the PF activation series to
implement station cleanup in the unregister handler.
> if (enic_is_sriov_vf_v2(enic)) {
> + enic_vf_admin_mac_quiesce(enic);
> +
> if (READ_ONCE(enic->vf_registered)) {
> int unreg_err = enic_mbox_vf_unregister(enic);
[Severity: Medium]
At this commit, enic_vf_admin_mac_quiesce(), VF_UNREGISTER and
enic_admin_channel_close() all run while the netdev is still registered
and attached, and RTNL is not held. Can a concurrent ndo_open undo this
teardown?
Under RTNL, enic_open() can see !admin_chan_up, !vf_registered or
reconnect_required and reopen the channel:
if (enic->admin_chan_up)
enic_admin_channel_close(enic);
err = enic_admin_chan_reopen(enic);
That path re-registers with the PF and then rearms the worker:
enic_open()
enic_admin_chan_reopen()
... admin channel open, VF register ...
enic_vf_admin_mac_rearm()
enic->vf_admin_mac_work_enabled = true;
unregister_netdev() -> enic_stop() does not close the admin channel.
Could the admin resources and the admin IRQ then outlive
free_netdev()?
The last patch in the series, "enic: configure V2 VF receive mode over
mailbox", closes this window:
- It adds rtnl_lock(); netif_device_detach(netdev); rtnl_unlock();
after the quiesce, so __dev_open() returns -ENODEV once the device
is detached.
- enic_admin_channel_close() quiesces the worker again at entry.
Could the detach be moved into this patch so intermediate commits don't
carry the race?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-b4-enic-sriov-v2-vf-receive-control-v2-0-5962c10061b2%40cisco.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net-next v2 6/6] enic: configure V2 VF receive mode over mailbox
2026-09-29 18:58 ` [PATCH net-next v2 6/6] enic: configure V2 VF receive mode over mailbox Satish Kharat
@ 2026-10-03 19:50 ` netdev-bot+sashiko
0 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-03 19:50 UTC (permalink / raw)
To: satishkh
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, netdev,
linux-kernel, sebaddel
Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 4 · Low: 2
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] enic_open() and enic_set_vf_rx_mode() handle the same PF refusal
of a needed broad receive mode in opposite ways.
- [Medium] For V2 VFs the patch stops syncing receive mode inside
enic_open(): enic_set_rx_mode() is now called only `if…
- [Medium] If the whole ADD_DEL_MAC request is rejected, the batch's
DELETEs are left unrecovered.
- [Medium] enic_vf_sync_mac_filters() misreads a transactional MAC-batch
result.
- [Medium] In enic_set_vf_rx_mode(), broad_enable_needed is computed from
what was previously requested, not from what was applied: `(flags &…
- [Low] The comment in the broad_enable_needed branch of
enic_set_vf_rx_mode() says the code will 'Retain any broad mode that…
- [Low] enic_set_vf_rx_mode() runs from the global netdev_work item under
rtnl_lock(): enic has no ops lock, so netdev_lock_ops_compat() takes…
--
enic: configure V2 VF receive mode over mailbox
This patch adds an ndo_set_rx_mode_async callback for V2 VFs. The callback
sends unicast/multicast exact-filter changes and packet-filter settings to
the PF over the mailbox. For V2 VFs, enic_open() now programs only the
packet filter and relies on the core async receive-mode path (scheduled
explicitly after internal resets) to replay the address lists.
> diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
> index 2132bfa9c8d38..382bac6669bf3 100644
> --- a/drivers/net/ethernet/cisco/enic/enic_main.c
> +++ b/drivers/net/ethernet/cisco/enic/enic_main.c
[ ... ]
> +static int enic_vf_sync_mac_filters(struct enic *enic,
> + struct netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc,
> + bool install_uc, bool install_mc)
> +{
[ ... ]
> + /* The protocol carries all changed unicast and multicast addresses in
> + * one request. Besides matching the native ABI, this bounds time spent
> + * in the RTNL-held asynchronous receive-mode callback to one MAC reply.
> + */
[Severity: Low]
This isn't a bug, but does this comment describe the whole callback? The
ADD_DEL_MAC part is bounded to one reply. Before calling this,
enic_set_vf_rx_mode() can also send the broad_enable_needed SET_PKT_FILTER
and the broad_withdrawal SET_PKT_FILTER. Each of those waits up to 3000 ms
in enic_mbox_wait_reply().
The callback runs from netdev_work_proc() with RTNL held through
netdev_lock_ops_compat(). So one callback can hold RTNL across up to three
mailbox round trips.
Also, while PROMISC or ALLMULTI is requested, filter_needed is always true.
Every receive-mode run then sends a SET_PKT_FILTER under RTNL.
A timeout sets the reconnect flag, so later sends fail fast. The commit
message also says completing all phases in one callback is deliberate.
Could the comment say that the bound applies to the MAC request only?
> + err = enic_mbox_vf_add_del_macs(enic, macs, num_ops);
> + if (err)
> + goto free_ops;
[Severity: Medium]
What happens to the DELETEs in this batch if the PF rejects the whole
ADD_DEL_MAC request?
For a nonzero ret_major such as ENIC_MBOX_ERR_GENERIC, or an errno-style
value, enic_mbox_vf_classify_reply() returns -EIO and leaves recovery at
ENIC_MBOX_VF_REPLY_OK:
if (ret_major)
return -EIO;
The SET_PKT_FILTER reply handler escalates this case to a reconnect, but
the ADD_DEL_MAC handler does not. So enic_mbox_vf_add_del_macs() returns
-EIO without calling enic_mbox_vf_require_reconnect().
This goto free_ops then skips the per-address loop below. That loop is the
only place where a failed DELETE becomes a reconnect. The comment further
down says:
A failed DELETE can leave hardware accepting an address no longer in
the requested list. Registration is the fail-closed cleanup boundary.
enic_set_vf_rx_mode() returns -EIO. The core retries at most
NETIF_RX_MODE_RETRY_MAX times, and then netif_rx_mode_schedule_retry()
prints "rx_mode retry limit reached, giving up".
After that, can the removed address stay installed at the PF, still
accepting traffic, until some unrelated receive-mode event?
> +
> + for (i = 0; i < num_ops; i++) {
> + u16 flags = le16_to_cpu(macs[i].flags);
> + bool add = flags & ENIC_MAC_ADDR_FLAG_ADD;
> +
> + if (flags & ENIC_MAC_ADDR_FLAG_SKIPPED) {
> + if (add)
> + retryable_add = true;
> + else
> + reconnect = true;
> + continue;
> + }
[ ... ]
> + if (reconnect) {
> + enic_mbox_vf_require_reconnect(enic);
> + err = -EIO;
> + } else if (retryable_add) {
> + err = -EAGAIN;
> + } else if (permanent_add) {
> + err = -EACCES;
> + }
[Severity: Medium]
How does this classification behave when the PF permanently rejects one ADD
and marks the rest of the batch as SKIPPED? enic_vf_station_addr_replace()
documents that PF behaviour:
/* A policy-rejected ADD can be reported with all preceding DELETEs
* skipped. No operation changed state in that coherent result tuple.
*/
Here a SKIPPED DELETE sets reconnect. One stable policy denial then calls
enic_mbox_vf_require_reconnect() and triggers a VF reset on every list
change that includes a DELETE.
The rejected ADD keeps sync_cnt == 0, so enic_vf_collect_mac_ops() sends it
again in every batch. After the reset replay the batch contains only ADDs,
and the valid ADDs come back SKIPPED. Because retryable_add is checked
before permanent_add, the result is -EAGAIN rather than -EACCES.
Does the core then retry the same batch until NETIF_RX_MODE_RETRY_MAX and
give up, leaving none of the valid secondary addresses installed? That
seems to contradict the commit message:
stable policy denials do not consume the retry budget
The -EAGAIN/-EIO return also skips the final enic_vf_set_pkt_filter() in
enic_set_vf_rx_mode(), so pending packet-filter changes are dropped too.
[ ... ]
> +static int enic_set_vf_rx_mode(struct net_device *netdev,
> + struct netdev_hw_addr_list *uc,
> + struct netdev_hw_addr_list *mc)
> +{
[ ... ]
> + filter_needed = !enic->vf_pkt_filter_valid ||
> + enic->vf_pkt_filter_requested != flags ||
> + (flags & broad_modes);
> + broad_enable_needed =
> + (!enic->vf_pkt_filter_valid && (flags & broad_modes)) ||
> + (flags & broad_modes & ~enic->vf_pkt_filter_requested);
> + broad_withdrawal = enic->vf_pkt_filter_valid &&
> + (enic->vf_pkt_filter_applied & broad_modes & ~flags);
> + if (broad_enable_needed) {
> + /* Establish newly required broad coverage before an independent
> + * exact-address rejection can block it. Retain any broad mode that
> + * is currently applied until its finite replacement is installed.
> + */
> + prefilter_flags = flags;
> + if (enic->vf_pkt_filter_valid)
> + prefilter_flags |= enic->vf_pkt_filter_applied & broad_modes;
> + err = enic_vf_set_pkt_filter(enic, prefilter_flags,
> + &applied_flags);
> + if (err)
> + return err;
> + target_filter_updated = prefilter_flags == flags;
> + enic_vf_report_pkt_filter_denial(netdev, flags, applied_flags);
> + }
> +
> + if (broad_withdrawal) {
> + /* Withdraw stale broad acceptance before installing its finite
> + * replacement. Complete both phases in this callback so a synchronous
> + * receive-mode operation cannot return between them.
> + */
> + err = enic_vf_set_pkt_filter(enic, flags, &applied_flags);
[Severity: Low]
This isn't a bug, but the broad_enable_needed comment above says the
applied broad mode is kept until its finite replacement is installed.
When broad_withdrawal is also true, this block runs right afterwards. That
happens, for example, when UC goes from overflow back to finite while MC
starts to overflow. The block then removes the retained mode before
enic_vf_sync_mac_filters() installs the exact filters.
The commit message and this comment describe the actual order. Should the
earlier comment be updated? In this case the retained bit in
prefilter_flags only costs an extra mailbox round trip.
[ ... ]
> + err = enic_vf_sync_mac_filters(enic, uc, mc, !uc_overflow,
> + !mc_overflow);
> + if (err == -EACCES) {
[ ... ]
> + }
> + if (err)
> + return err;
> +
> + if (!target_filter_updated && filter_needed) {
> + err = enic_vf_set_pkt_filter(enic, flags, &applied_flags);
> + if (err)
> + return err;
[Severity: Medium]
Can a denied broad mode stay unrequested after PF policy starts allowing
it? broad_enable_needed is computed from vf_pkt_filter_requested, not from
vf_pkt_filter_applied:
(flags & broad_modes & ~enic->vf_pkt_filter_requested);
Once PROMISCUOUS or ALL_MULTICAST has been requested and withheld, the bit
is set in vf_pkt_filter_requested, so later callbacks skip the early enable
phase. The broad mode is then re-requested only by this final
enic_vf_set_pkt_filter().
If a finite-class ADD comes back SKIPPED (-EAGAIN), or the MAC request
fails some other way, the "if (err) return err;" above returns before this
call. Repeated failures can use up the core retry budget without the
broad-mode request ever being sent again.
That looks like the case the broad_enable_needed comment says the code
prevents:
Establish newly required broad coverage before an independent
exact-address rejection can block it.
[ ... ]
> @@ -2331,9 +2703,38 @@ static int enic_open(struct net_device *netdev)
> enic_vf_station_addr_set(enic, netdev->dev_addr);
> enic_vf_station_sync_reset(enic);
> vf_mac_added = true;
[ ... ]
> + if ((enic_vf_uc_needs_promisc(vf_uc_count) &&
> + !(vf_filter_applied & CMD_PFILTER_PROMISCUOUS)) ||
> + (vf_mc_count > ENIC_MULTICAST_PERFECT_FILTERS &&
> + !(vf_filter_applied & CMD_PFILTER_ALL_MULTICAST))) {
> + netdev_err(netdev,
> + "PF denied receive mode required by VF address lists\n");
> + err = -EACCES;
> + goto err_out_disable_wq;
> + }
> + }
[Severity: High]
Is it intended that enic_open() fails on a denial that
enic_set_vf_rx_mode() accepts at runtime?
Take 32 or more secondary UC addresses, or more than 32 MC addresses, with
a PF policy that withholds PROMISCUOUS/ALL_MULTICAST (for example an
untrusted VF). enic_set_vf_rx_mode() computes the same flags, but it:
- only reports the denial with netdev_dbg() in
enic_vf_report_pkt_filter_denial()
- stops installing new exact filters for the overflowing class
- returns 0
So the lists can grow past the limit while the interface is up, and the VF
keeps running while silently dropping the extra traffic.
Any later enic_open() with those lists then fails here with -EACCES. That
includes an administrative down/up, _enic_change_mtu(), enic_reset() and
enic_tx_hang_reset(). The reset workers only log the error:
err = enic_open(enic->netdev);
if (err)
netdev_err(enic->netdev,
"Failed to reopen datapath after reset: %d\n", err);
Does that leave the VF with IFF_UP set and the datapath closed until the
address lists are shrunk? The commit message doesn't mention this new
failure mode for open and reset.
>
> - enic_set_rx_mode(netdev);
> + if (!enic_is_sriov_vf_v2(enic))
> + enic_set_rx_mode(netdev);
[Severity: Medium]
enic_set_rx_mode() is now skipped for V2 VFs. Every caller that calls
enic_open() directly, bypassing __dev_open(), then has to schedule the
async update itself. enic_reset() and enic_tx_hang_reset() do that below.
Is _enic_change_mtu() missing the same call?
enic_mtu_check()->schedule_work(&enic->change_mtu_work)
enic_change_mtu_work()
_enic_change_mtu()
enic_open()
_enic_change_mtu() {
...
if (running) {
err = enic_open(netdev);
if (err)
return err;
}
...
}
Here enic_open() can take its reconnect branch (!admin_chan_up,
!vf_registered or vf_mbox_reconnect_required). That branch is reachable
after a failed reset handshake, because vf_mbox_reconnect_required stays
set until a later external recovery event.
That branch calls enic_reset_addr_lists() and re-registers, so the PF drops
all VF-requested filters. Only the station address and the packet filter
are then installed again.
Would the secondary unicast and multicast filters then stay uninstalled
until some unrelated address-list or flag change queues a receive-mode
run?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-b4-enic-sriov-v2-vf-receive-control-v2-0-5962c10061b2%40cisco.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-10-03 19:50 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-29 18:58 [PATCH net-next v2 0/6] enic: configure V2 VF addresses and receive mode Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 1/6] net: add netif_rx_mode_schedule_update() Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 2/6] enic: serialize V2 VF mailbox requests Satish Kharat
2026-09-29 18:58 ` [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
2026-09-29 18:58 ` [PATCH net-next v2 4/6] enic: validate V2 VF configuration replies Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
2026-09-29 18:58 ` [PATCH net-next v2 5/6] enic: manage V2 VF station and administrative MAC Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
2026-09-29 18:58 ` [PATCH net-next v2 6/6] enic: configure V2 VF receive mode over mailbox Satish Kharat
2026-10-03 19:50 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®