enic: match mailbox replies to request numbers

The version-1 VF mailbox protocol identifies every message with a message
number, and a reply or acknowledgment echoes the number of the message it
answers.  ENIC instead generates a new number for outgoing replies and
accepts a VF reply by message type alone.

If a request times out, a delayed reply can therefore satisfy a subsequent
request of the same type and cause the VF to consume the result of the old
request.

Allow replies to reuse the initiating message number. Make the in-tree PF
handlers and the VF link-state acknowledgment echo that number. Record the
expected reply type and message number on the VF, and require both values
to match before accepting a reply.

Protect expected-reply state with a lock so reply acceptance and timeout
invalidation cannot race. Keep message numbers monotonic across an admin-
channel reopen so a delayed reply from an earlier channel generation cannot
match a new request.

Reply-number echo is part of the established version-1 protocol, so this
remains compatible with deployed V2-capable PF implementations that already
echo msg_num.

Fixes: 72b65c9405 ("enic: add MBOX VF handlers for capability, register and link state")
Signed-off-by: Satish Kharat <satishkh@cisco.com>
Link: https://patch.msgid.link/20260830-b4-enic-v2-mbox-fixes-net-v1-2-23adf9bfd426@cisco.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Satish Kharat 2026-08-30 15:22:53 -07:00 committed by Jakub Kicinski
parent b752e041d5
commit 8972d252f4
3 changed files with 180 additions and 130 deletions

View File

@ -329,15 +329,14 @@ struct enic {
/* MBOX protocol state — mbox_lock serializes admin WQ sends */
struct mutex mbox_lock;
u64 mbox_msg_num;
/* MBOX request-reply state. mbox_expected_reply is written and
* cleared by the process-context request helpers (capability/register/
* unregister) and only read by the admin_msg_work receive handlers, so
* it is annotated with READ_ONCE()/WRITE_ONCE() rather than locked:
* only one request is in flight at a time (requesters run under RTNL or
* single-threaded probe/remove), so each request is serialized and its
* reply completes mbox_comp before the next request is issued.
/* MBOX request-reply state. Existing request callers allow only one
* request in flight. The state lock arbitrates reply acceptance against
* timeout invalidation, while mbox_comp publishes the accepted result to
* the requester.
*/
struct completion mbox_comp;
spinlock_t mbox_state_lock; /* protects expected reply state */
u64 mbox_expected_msg_num;
u8 mbox_expected_reply;
bool mbox_initialized;

View File

@ -2206,9 +2206,10 @@ static void enic_admin_chan_reopen(struct enic *enic)
{
int err;
/* Install the MBOX receive handler and reset the sequence number
* before opening the channel, so the handler is in place before the
* admin interrupt is unmasked and no early completion is dropped.
/* Install the MBOX receive handler and clear pending reply state before
* opening the channel, so the handler is in place before the admin
* interrupt is unmasked and no early completion is dropped. Keep the
* sequence number monotonic across channel generations.
*/
enic_mbox_init(enic);
@ -2220,7 +2221,7 @@ static void enic_admin_chan_reopen(struct enic *enic)
* registration over a dead channel.
*/
if (enic_is_sriov_vf_v2(enic))
enic->vf_registered = false;
WRITE_ONCE(enic->vf_registered, false);
err = enic_admin_channel_open(enic);
if (err) {
@ -3349,7 +3350,7 @@ static int enic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err_out_admin_close:
if (enic_is_sriov_vf_v2(enic)) {
if (enic->vf_registered) {
if (READ_ONCE(enic->vf_registered)) {
int unreg_err = enic_mbox_vf_unregister(enic);
if (unreg_err)
@ -3402,7 +3403,7 @@ static void enic_remove(struct pci_dev *pdev)
* touching a netdev that is being torn down.
*/
if (enic_is_sriov_vf_v2(enic)) {
if (enic->vf_registered) {
if (READ_ONCE(enic->vf_registered)) {
int unreg_err = enic_mbox_vf_unregister(enic);
if (unreg_err)

View File

@ -18,22 +18,25 @@
#define ENIC_MBOX_POLL_TIMEOUT_US 5000000
#define ENIC_MBOX_POLL_INTERVAL_US 100
static void enic_mbox_fill_hdr(struct enic *enic, struct enic_mbox_hdr *hdr,
u8 msg_type, u16 dst_vnic_id, u16 msg_len)
static void enic_mbox_fill_hdr(struct enic_mbox_hdr *hdr, u8 msg_type,
u16 dst_vnic_id, u16 msg_len, u64 msg_num)
{
memset(hdr, 0, sizeof(*hdr));
hdr->dst_vnic_id = cpu_to_le16(dst_vnic_id);
hdr->msg_type = msg_type;
hdr->msg_len = cpu_to_le16(msg_len);
hdr->msg_num = cpu_to_le64(++enic->mbox_msg_num);
hdr->msg_num = cpu_to_le64(msg_num);
}
int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
void *payload, u16 payload_len)
static int enic_mbox_send_msg_id(struct enic *enic, u8 msg_type,
u16 dst_vnic_id, void *payload,
u16 payload_len, u64 msg_num, bool reuse_msg_num,
u8 expected_reply)
{
size_t total_len = sizeof(struct enic_mbox_hdr) + payload_len;
struct vnic_wq *wq = &enic->admin_wq;
struct wq_enet_desc *desc;
bool reply_expected = false;
unsigned long timeout;
dma_addr_t dma_addr;
u16 vlan_tag;
@ -68,7 +71,21 @@ int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
goto unlock;
}
enic_mbox_fill_hdr(enic, buf, msg_type, dst_vnic_id, total_len);
/* Replies reuse the initiating message number. Requests and
* notifications allocate a new one.
*/
if (!reuse_msg_num)
msg_num = ++enic->mbox_msg_num;
if (expected_reply) {
reinit_completion(&enic->mbox_comp);
spin_lock_bh(&enic->mbox_state_lock);
enic->mbox_expected_reply = expected_reply;
enic->mbox_expected_msg_num = msg_num;
spin_unlock_bh(&enic->mbox_state_lock);
reply_expected = true;
}
enic_mbox_fill_hdr(buf, msg_type, dst_vnic_id, total_len, msg_num);
if (payload_len) {
void *dst = buf + sizeof(struct enic_mbox_hdr);
@ -139,18 +156,66 @@ int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
"MBOX send msg_type %u dst %u vlan %u err %d\n",
msg_type, dst_vnic_id, vlan_tag, err);
unlock:
if (err && reply_expected) {
spin_lock_bh(&enic->mbox_state_lock);
if (enic->mbox_expected_reply == expected_reply &&
enic->mbox_expected_msg_num == msg_num) {
enic->mbox_expected_reply = 0;
enic->mbox_expected_msg_num = 0;
}
spin_unlock_bh(&enic->mbox_state_lock);
}
mutex_unlock(&enic->mbox_lock);
return err;
}
int enic_mbox_send_msg(struct enic *enic, u8 msg_type, u16 dst_vnic_id,
void *payload, u16 payload_len)
{
return enic_mbox_send_msg_id(enic, msg_type, dst_vnic_id, payload,
payload_len, 0, false, 0);
}
static int enic_mbox_send_reply(struct enic *enic, u8 msg_type,
u16 dst_vnic_id, void *payload, u16 payload_len,
u64 msg_num)
{
return enic_mbox_send_msg_id(enic, msg_type, dst_vnic_id, payload,
payload_len, msg_num, true, 0);
}
static int enic_mbox_vf_send_request(struct enic *enic, u8 request_type,
u8 expected_reply, void *payload,
u16 payload_len)
{
return enic_mbox_send_msg_id(enic, request_type, ENIC_MBOX_DST_PF,
payload, payload_len, 0, false,
expected_reply);
}
static int enic_mbox_wait_reply(struct enic *enic, unsigned long timeout_ms)
{
unsigned long left;
int err = 0;
left = wait_for_completion_timeout(&enic->mbox_comp,
msecs_to_jiffies(timeout_ms));
if (left)
return 0;
return left ? 0 : -ETIMEDOUT;
/* 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.
*/
spin_lock_bh(&enic->mbox_state_lock);
if (enic->mbox_expected_reply) {
enic->mbox_expected_reply = 0;
enic->mbox_expected_msg_num = 0;
err = -ETIMEDOUT;
}
spin_unlock_bh(&enic->mbox_state_lock);
return err;
}
int enic_mbox_send_link_state(struct enic *enic, u16 vf_id, u32 link_state)
@ -178,8 +243,8 @@ static int enic_mbox_pf_handle_capability(struct enic *enic, void *msg,
reply.reply.ret_major = cpu_to_le16(0);
reply.version = cpu_to_le32(ENIC_MBOX_CAP_VERSION_1);
return enic_mbox_send_msg(enic, ENIC_MBOX_VF_CAPABILITY_REPLY, vf_id,
&reply, sizeof(reply));
return enic_mbox_send_reply(enic, ENIC_MBOX_VF_CAPABILITY_REPLY, vf_id,
&reply, sizeof(reply), msg_num);
}
static int enic_mbox_pf_handle_register(struct enic *enic, void *msg,
@ -208,8 +273,8 @@ static int enic_mbox_pf_handle_register(struct enic *enic, void *msg,
}
reply.reply.ret_major = cpu_to_le16(0);
err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_REGISTER_REPLY, vf_id,
&reply, sizeof(reply));
err = enic_mbox_send_reply(enic, ENIC_MBOX_VF_REGISTER_REPLY, vf_id,
&reply, sizeof(reply), msg_num);
if (err)
return err;
@ -253,8 +318,8 @@ static int enic_mbox_pf_handle_unregister(struct enic *enic, void *msg,
enic->vf_state[vf_id].registered = false;
reply.reply.ret_major = cpu_to_le16(0);
err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_UNREGISTER_REPLY, vf_id,
&reply, sizeof(reply));
err = enic_mbox_send_reply(enic, ENIC_MBOX_VF_UNREGISTER_REPLY, vf_id,
&reply, sizeof(reply), msg_num);
if (net_ratelimit())
netdev_info(enic->netdev,
@ -324,75 +389,57 @@ static void enic_mbox_pf_process_msg(struct enic *enic,
hdr->msg_type, vf_id, err);
}
static void enic_mbox_vf_handle_capability_reply(struct enic *enic,
void *payload)
static void enic_mbox_vf_handle_reply(struct enic *enic, u8 reply_type,
void *payload, u64 msg_num)
{
struct enic_mbox_vf_capability_reply_msg *reply = payload;
struct enic_mbox_generic_reply *reply = payload;
u16 ret_major = le16_to_cpu(reply->ret_major);
u64 expected_msg_num;
u8 expected_type;
if (READ_ONCE(enic->mbox_expected_reply) != ENIC_MBOX_VF_CAPABILITY_REPLY) {
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);
netdev_warn(enic->netdev,
"MBOX: stale capability reply (expected %u), drop\n",
READ_ONCE(enic->mbox_expected_reply));
"MBOX: stale reply %u/%llu (expected %u/%llu), drop\n",
reply_type, (unsigned long long)msg_num,
expected_type, (unsigned long long)expected_msg_num);
return;
}
if (le16_to_cpu(reply->reply.ret_major) == 0)
enic->pf_cap_version = le32_to_cpu(reply->version);
else
netdev_warn(enic->netdev,
"MBOX: PF rejected capability request: %u/%u\n",
le16_to_cpu(reply->reply.ret_major),
le16_to_cpu(reply->reply.ret_minor));
if (!ret_major) {
switch (reply_type) {
case ENIC_MBOX_VF_CAPABILITY_REPLY: {
struct enic_mbox_vf_capability_reply_msg *cap = payload;
WRITE_ONCE(enic->pf_cap_version,
le32_to_cpu(cap->version));
break;
}
case ENIC_MBOX_VF_REGISTER_REPLY:
WRITE_ONCE(enic->vf_registered, true);
break;
case ENIC_MBOX_VF_UNREGISTER_REPLY:
WRITE_ONCE(enic->vf_registered, false);
break;
}
}
enic->mbox_expected_reply = 0;
enic->mbox_expected_msg_num = 0;
complete(&enic->mbox_comp);
spin_unlock_bh(&enic->mbox_state_lock);
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));
}
static void enic_mbox_vf_handle_register_reply(struct enic *enic,
void *payload)
{
struct enic_mbox_vf_register_reply_msg *reply = payload;
if (READ_ONCE(enic->mbox_expected_reply) != ENIC_MBOX_VF_REGISTER_REPLY) {
netdev_warn(enic->netdev,
"MBOX: stale register reply (expected %u), drop\n",
READ_ONCE(enic->mbox_expected_reply));
return;
}
if (le16_to_cpu(reply->reply.ret_major)) {
netdev_warn(enic->netdev,
"MBOX: VF register rejected by PF: %u/%u\n",
le16_to_cpu(reply->reply.ret_major),
le16_to_cpu(reply->reply.ret_minor));
} else {
enic->vf_registered = true;
}
complete(&enic->mbox_comp);
}
static void enic_mbox_vf_handle_unregister_reply(struct enic *enic,
void *payload)
{
struct enic_mbox_vf_register_reply_msg *reply = payload;
if (READ_ONCE(enic->mbox_expected_reply) != ENIC_MBOX_VF_UNREGISTER_REPLY) {
netdev_warn(enic->netdev,
"MBOX: stale unregister reply (expected %u), drop\n",
READ_ONCE(enic->mbox_expected_reply));
return;
}
if (le16_to_cpu(reply->reply.ret_major)) {
netdev_warn(enic->netdev,
"MBOX: VF unregister rejected by PF: %u/%u\n",
le16_to_cpu(reply->reply.ret_major),
le16_to_cpu(reply->reply.ret_minor));
} else {
enic->vf_registered = false;
}
complete(&enic->mbox_comp);
}
static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload)
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 = {};
@ -423,8 +470,8 @@ static void enic_mbox_vf_handle_link_state(struct enic *enic, void *payload)
}
spin_unlock_bh(&enic->vf_link_state_lock);
err = enic_mbox_send_msg(enic, ENIC_MBOX_PF_LINK_STATE_ACK,
ENIC_MBOX_DST_PF, &ack, sizeof(ack));
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);
@ -468,6 +515,8 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
struct enic_mbox_hdr *hdr, void *payload,
u16 payload_len)
{
u64 msg_num = le64_to_cpu(hdr->msg_num);
switch (hdr->msg_type) {
case ENIC_MBOX_VF_CAPABILITY_REPLY: {
size_t exp = sizeof(struct enic_mbox_vf_capability_reply_msg);
@ -475,7 +524,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
payload_len, exp))
return;
enic_mbox_vf_handle_capability_reply(enic, payload);
enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
break;
}
case ENIC_MBOX_VF_REGISTER_REPLY: {
@ -484,7 +533,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
payload_len, exp))
return;
enic_mbox_vf_handle_register_reply(enic, payload);
enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
break;
}
case ENIC_MBOX_VF_UNREGISTER_REPLY: {
@ -493,7 +542,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
payload_len, exp))
return;
enic_mbox_vf_handle_unregister_reply(enic, payload);
enic_mbox_vf_handle_reply(enic, hdr->msg_type, payload, msg_num);
break;
}
case ENIC_MBOX_PF_LINK_STATE_NOTIF: {
@ -502,7 +551,7 @@ static void enic_mbox_vf_process_msg(struct enic *enic,
if (!enic_mbox_vf_payload_ok(enic, hdr->msg_type,
payload_len, exp))
return;
enic_mbox_vf_handle_link_state(enic, payload);
enic_mbox_vf_handle_link_state(enic, payload, msg_num);
break;
}
default:
@ -571,32 +620,31 @@ static void enic_mbox_recv_handler(struct enic *enic, void *buf,
int enic_mbox_vf_capability_check(struct enic *enic)
{
struct enic_mbox_vf_capability_msg req = {};
u32 version;
int err;
enic->pf_cap_version = 0;
reinit_completion(&enic->mbox_comp);
WRITE_ONCE(enic->mbox_expected_reply, ENIC_MBOX_VF_CAPABILITY_REPLY);
WRITE_ONCE(enic->pf_cap_version, 0);
req.version = cpu_to_le32(ENIC_MBOX_CAP_VERSION_1);
err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_CAPABILITY_REQUEST,
ENIC_MBOX_DST_PF, &req, sizeof(req));
if (err) {
WRITE_ONCE(enic->mbox_expected_reply, 0);
err = enic_mbox_vf_send_request(enic,
ENIC_MBOX_VF_CAPABILITY_REQUEST,
ENIC_MBOX_VF_CAPABILITY_REPLY,
&req, sizeof(req));
if (err)
return err;
}
err = enic_mbox_wait_reply(enic, 3000);
WRITE_ONCE(enic->mbox_expected_reply, 0);
version = READ_ONCE(enic->pf_cap_version);
if (err) {
netdev_warn(enic->netdev,
"MBOX: no capability reply from PF\n");
return err;
}
if (enic->pf_cap_version < ENIC_MBOX_CAP_VERSION_1) {
if (version < ENIC_MBOX_CAP_VERSION_1) {
netdev_warn(enic->netdev,
"MBOX: PF rejected capability request or reported unsupported version %u\n",
enic->pf_cap_version);
version);
return -EOPNOTSUPP;
}
@ -605,28 +653,25 @@ int enic_mbox_vf_capability_check(struct enic *enic)
int enic_mbox_vf_register(struct enic *enic)
{
bool registered;
int err;
enic->vf_registered = false;
reinit_completion(&enic->mbox_comp);
WRITE_ONCE(enic->mbox_expected_reply, ENIC_MBOX_VF_REGISTER_REPLY);
WRITE_ONCE(enic->vf_registered, false);
err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_REGISTER_REQUEST,
ENIC_MBOX_DST_PF, NULL, 0);
if (err) {
WRITE_ONCE(enic->mbox_expected_reply, 0);
err = enic_mbox_vf_send_request(enic, ENIC_MBOX_VF_REGISTER_REQUEST,
ENIC_MBOX_VF_REGISTER_REPLY, NULL, 0);
if (err)
return err;
}
err = enic_mbox_wait_reply(enic, 3000);
WRITE_ONCE(enic->mbox_expected_reply, 0);
registered = READ_ONCE(enic->vf_registered);
if (err) {
netdev_warn(enic->netdev,
"MBOX: VF registration with PF timed out\n");
return err;
}
if (!enic->vf_registered)
if (!registered)
return -ENODEV;
return 0;
@ -634,43 +679,48 @@ int enic_mbox_vf_register(struct enic *enic)
int enic_mbox_vf_unregister(struct enic *enic)
{
bool registered;
int err;
if (!enic->vf_registered)
if (!READ_ONCE(enic->vf_registered))
return 0;
reinit_completion(&enic->mbox_comp);
WRITE_ONCE(enic->mbox_expected_reply, ENIC_MBOX_VF_UNREGISTER_REPLY);
err = enic_mbox_send_msg(enic, ENIC_MBOX_VF_UNREGISTER_REQUEST,
ENIC_MBOX_DST_PF, NULL, 0);
if (err) {
WRITE_ONCE(enic->mbox_expected_reply, 0);
return err;
}
err = enic_mbox_wait_reply(enic, 3000);
WRITE_ONCE(enic->mbox_expected_reply, 0);
err = enic_mbox_vf_send_request(enic,
ENIC_MBOX_VF_UNREGISTER_REQUEST,
ENIC_MBOX_VF_UNREGISTER_REPLY,
NULL, 0);
if (err)
return err;
if (enic->vf_registered)
err = enic_mbox_wait_reply(enic, 3000);
registered = READ_ONCE(enic->vf_registered);
if (err)
return err;
if (registered)
return -EACCES;
return 0;
}
void enic_mbox_init(struct enic *enic)
{
/* mbox_lock and mbox_comp must be initialized exactly once per
bool reinit = enic->mbox_initialized;
/* MBOX locks and mbox_comp must be initialized exactly once per
* device lifetime; the PF sriov_configure path can re-enter this
* on each enable cycle where these primitives are already set up.
*/
if (!enic->mbox_initialized) {
if (!reinit) {
mutex_init(&enic->mbox_lock);
init_completion(&enic->mbox_comp);
spin_lock_init(&enic->mbox_state_lock);
enic->mbox_msg_num = 0;
enic->mbox_initialized = true;
} else {
reinit_completion(&enic->mbox_comp);
}
enic->mbox_msg_num = 0;
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);
enic->admin_rq_handler = enic_mbox_recv_handler;
}