* Re: [PATCH v11 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Peter Zijlstra @ 2025-11-12 9:37 UTC (permalink / raw)
To: Michael Kelley
Cc: Naman Jain, Paolo Bonzini, Sean Christopherson,
K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H . Peter Anvin, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, x86@kernel.org, Mukesh Rathor,
Stanislav Kinsburskii, Nuno Das Neves, Christoph Hellwig,
Saurabh Sengar, ALOK TIWARI
In-Reply-To: <SN6PR02MB4157C399DB7624C28D0860AAD4CCA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Wed, Nov 12, 2025 at 04:12:08AM +0000, Michael Kelley wrote:
> > @@ -96,3 +97,10 @@ SYM_FUNC_START(__mshv_vtl_return_call)
> > pop %rbp
> > RET
> > SYM_FUNC_END(__mshv_vtl_return_call)
> > +
> > + .section .discard.addressable,"aw"
> > + .align 8
> > + .type __UNIQUE_ID_addressable___SCK____mshv_vtl_return_hypercall_662.0, @object
> > + .size __UNIQUE_ID_addressable___SCK____mshv_vtl_return_hypercall_662.0, 8
> > +__UNIQUE_ID_addressable___SCK____mshv_vtl_return_hypercall_662.0:
> > + .quad __SCK____mshv_vtl_return_hypercall
>
> This is pretty yucky itself.
Definitely doesn't win any prizes, for sure.
> Why is it better than calling out to a C function?
It keeps all the code in one place is a strong argument.
> Is it because in spite of the annotations, there's no guarantee the C
> compiler won't generate some code that messes up a register value? Or is
> there some other reason?
There is that too, a frame pointer build would be in its right to add a
stack frame (although they typically won't in this case). And the C ABI
doesn't provide the guarantees your need, so calling out into C is very
much you get to keep the pieces.
> Does the magic "_662.0" have any significance? Or is it just some
> uniqueness salt on the symbol name?
Like Paolo already said, that's just the crazy generated by our
__ADRESSABLE() macro, this name is mostly irrelevant, all we really need
is a reference to that __SCK____mshv_vtl_return_hypercall symbol so it
ends up in the symbol table. (And the final link will then complain if
the symbol doesn't end up being resolved)
Keeping the name somewhat in line with __ADDRESSABLE() has the advantage
that you can clearly see where it comes from, but yeah, we can strip of
the number if you like.
^ permalink raw reply
* RE: [PATCH v11 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Michael Kelley @ 2025-11-12 9:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Naman Jain, Paolo Bonzini, Sean Christopherson,
K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H . Peter Anvin, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, x86@kernel.org, Mukesh Rathor,
Stanislav Kinsburskii, Nuno Das Neves, Christoph Hellwig,
Saurabh Sengar, ALOK TIWARI
In-Reply-To: <20251112093704.GC4067720@noisy.programming.kicks-ass.net>
From: Peter Zijlstra <peterz@infradead.org> Sent: Wednesday, November 12, 2025 1:37 AM
>
> On Wed, Nov 12, 2025 at 04:12:08AM +0000, Michael Kelley wrote:
>
> > > @@ -96,3 +97,10 @@ SYM_FUNC_START(__mshv_vtl_return_call)
> > > pop %rbp
> > > RET
> > > SYM_FUNC_END(__mshv_vtl_return_call)
> > > +
> > > + .section .discard.addressable,"aw"
> > > + .align 8
> > > + .type __UNIQUE_ID_addressable___SCK____mshv_vtl_return_hypercall_662.0, @object
> > > + .size __UNIQUE_ID_addressable___SCK____mshv_vtl_return_hypercall_662.0, 8
> > > +__UNIQUE_ID_addressable___SCK____mshv_vtl_return_hypercall_662.0:
> > > + .quad __SCK____mshv_vtl_return_hypercall
> >
> > This is pretty yucky itself.
>
> Definitely doesn't win any prizes, for sure.
>
> > Why is it better than calling out to a C function?
>
> It keeps all the code in one place is a strong argument.
>
> > Is it because in spite of the annotations, there's no guarantee the C
> > compiler won't generate some code that messes up a register value? Or is
> > there some other reason?
>
> There is that too, a frame pointer build would be in its right to add a
> stack frame (although they typically won't in this case). And the C ABI
> doesn't provide the guarantees your need, so calling out into C is very
> much you get to keep the pieces.
>
> > Does the magic "_662.0" have any significance? Or is it just some
> > uniqueness salt on the symbol name?
>
> Like Paolo already said, that's just the crazy generated by our
> __ADRESSABLE() macro, this name is mostly irrelevant, all we really need
> is a reference to that __SCK____mshv_vtl_return_hypercall symbol so it
> ends up in the symbol table. (And the final link will then complain if
> the symbol doesn't end up being resolved)
>
> Keeping the name somewhat in line with __ADDRESSABLE() has the advantage
> that you can clearly see where it comes from, but yeah, we can strip of
> the number if you like.
Thanks. If that symbol is referenced only by these few lines, I'd
go with something even shorter and simpler. Perhaps:
.section .discard.addressable,"aw"
.align 8
.type vtl_return_sym, @object
.size vtl_return_sym, 8
vtl_return_sym:
.quad __SCK____mshv_vtl_return_hypercall
Regardless of the choice of symbol name, add a comment about
mimicking __ADDRESSABLE(). That feels less messy to me, but
it's Naman's call.
Michael
^ permalink raw reply
* Re: [PATCH v11 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Peter Zijlstra @ 2025-11-12 10:10 UTC (permalink / raw)
To: Michael Kelley
Cc: Naman Jain, Paolo Bonzini, Sean Christopherson,
K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H . Peter Anvin, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, x86@kernel.org, Mukesh Rathor,
Stanislav Kinsburskii, Nuno Das Neves, Christoph Hellwig,
Saurabh Sengar, ALOK TIWARI
In-Reply-To: <SN6PR02MB4157F3F565621B32AC29EC92D4CCA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Wed, Nov 12, 2025 at 09:44:20AM +0000, Michael Kelley wrote:
> Thanks. If that symbol is referenced only by these few lines, I'd
> go with something even shorter and simpler. Perhaps:
>
> .section .discard.addressable,"aw"
> .align 8
> .type vtl_return_sym, @object
> .size vtl_return_sym, 8
> vtl_return_sym:
> .quad __SCK____mshv_vtl_return_hypercall
>
> Regardless of the choice of symbol name, add a comment about
> mimicking __ADDRESSABLE(). That feels less messy to me, but
> it's Naman's call.
Right, that'll work.
^ permalink raw reply
* Re: [PATCH v11 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-11-12 10:49 UTC (permalink / raw)
To: Peter Zijlstra, Michael Kelley, Paolo Bonzini
Cc: Paolo Bonzini, Sean Christopherson, K . Y . Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H . Peter Anvin,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
x86@kernel.org, Mukesh Rathor, Stanislav Kinsburskii,
Nuno Das Neves, Christoph Hellwig, Saurabh Sengar, ALOK TIWARI
In-Reply-To: <20251112101038.GE4067720@noisy.programming.kicks-ass.net>
On 11/12/2025 3:40 PM, Peter Zijlstra wrote:
> On Wed, Nov 12, 2025 at 09:44:20AM +0000, Michael Kelley wrote:
>
>> Thanks. If that symbol is referenced only by these few lines, I'd
>> go with something even shorter and simpler. Perhaps:
>>
>> .section .discard.addressable,"aw"
>> .align 8
>> .type vtl_return_sym, @object
>> .size vtl_return_sym, 8
>> vtl_return_sym:
>> .quad __SCK____mshv_vtl_return_hypercall
>>
>> Regardless of the choice of symbol name, add a comment about
>> mimicking __ADDRESSABLE(). That feels less messy to me, but
>> it's Naman's call.
>
> Right, that'll work.
Thanks Michael, Peter and Paolo for your inputs.
I'll use a simpler name and add a comment to explain it.
Regards,
Naman
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: mana: Drop TX skb on post_work_request failure and unmap resources
From: Aditya Garg @ 2025-11-12 12:12 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, longli, kotaranov, horms, shradhagupta, ssengar, ernis,
dipayanroy, shirazsaleem, leon, mlevitsk, yury.norov,
linux-hyperv, netdev, linux-kernel, linux-rdma, gargaditya
In-Reply-To: <20251111170837.602904ee@kernel.org>
On 12-11-2025 06:38, Jakub Kicinski wrote:
> On Tue, 11 Nov 2025 00:13:01 -0800 Aditya Garg wrote:
>> Drop TX packets when posting the work request fails and ensure DMA
>> mappings are always cleaned up.
>
> drivers/net/ethernet/microsoft/mana/gdma_main.c:1303:23: warning: variable 'gc' set but not used [-Wunused-but-set-variable]
> 1303 | struct gdma_context *gc;
> | ^
Thanks for pointing this out. Will fix this in next revision.
Regards,
Aditya
^ permalink raw reply
* [PATCH net-next v4 0/2] net: mana: Enforce TX SGE limit and fix error cleanup
From: Aditya Garg @ 2025-11-12 13:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, ssengar,
ernis, dipayanroy, shirazsaleem, leon, mlevitsk, yury.norov,
sbhatta, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya
Cc: Aditya Garg
Add pre-transmission checks to block SKBs that exceed the hardware's SGE
limit. Force software segmentation for GSO traffic and linearize non-GSO
packets as needed.
Update TX error handling to drop failed SKBs and unmap resources
immediately.
---
Changes in v4:
* Fix warning during build reported by kernel test robot
---
Aditya Garg (2):
net: mana: Handle SKB if TX SGEs exceed hardware limit
net: mana: Drop TX skb on post_work_request failure and unmap
resources
.../net/ethernet/microsoft/mana/gdma_main.c | 6 +--
drivers/net/ethernet/microsoft/mana/mana_en.c | 44 ++++++++++++++++---
.../ethernet/microsoft/mana/mana_ethtool.c | 2 +
include/net/mana/gdma.h | 6 ++-
include/net/mana/mana.h | 2 +
5 files changed, 48 insertions(+), 12 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH net-next v4 1/2] net: mana: Handle SKB if TX SGEs exceed hardware limit
From: Aditya Garg @ 2025-11-12 13:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, ssengar,
ernis, dipayanroy, shirazsaleem, leon, mlevitsk, yury.norov,
sbhatta, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya
Cc: Aditya Garg
In-Reply-To: <1762952506-23593-1-git-send-email-gargaditya@linux.microsoft.com>
The MANA hardware supports a maximum of 30 scatter-gather entries (SGEs)
per TX WQE. Exceeding this limit can cause TX failures.
Add ndo_features_check() callback to validate SKB layout before
transmission. For GSO SKBs that would exceed the hardware SGE limit, clear
NETIF_F_GSO_MASK to enforce software segmentation in the stack.
Add a fallback in mana_start_xmit() to linearize non-GSO SKBs that still
exceed the SGE limit.
Also, Add ethtool counter for SKBs linearized
Co-developed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 37 ++++++++++++++++++-
.../ethernet/microsoft/mana/mana_ethtool.c | 2 +
include/net/mana/gdma.h | 6 ++-
include/net/mana/mana.h | 1 +
4 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index cccd5b63cee6..67ae5421f9ee 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -11,6 +11,7 @@
#include <linux/mm.h>
#include <linux/pci.h>
#include <linux/export.h>
+#include <linux/skbuff.h>
#include <net/checksum.h>
#include <net/ip6_checksum.h>
@@ -329,6 +330,20 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
cq = &apc->tx_qp[txq_idx].tx_cq;
tx_stats = &txq->stats;
+ if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
+ skb_shinfo(skb)->nr_frags + 2 > MAX_TX_WQE_SGL_ENTRIES) {
+ /* GSO skb with Hardware SGE limit exceeded is not expected here
+ * as they are handled in mana_features_check() callback
+ */
+ if (skb_linearize(skb)) {
+ netdev_warn_once(ndev, "Failed to linearize skb with nr_frags=%d and is_gso=%d\n",
+ skb_shinfo(skb)->nr_frags,
+ skb_is_gso(skb));
+ goto tx_drop_count;
+ }
+ apc->eth_stats.linear_pkt_tx_cnt++;
+ }
+
pkg.tx_oob.s_oob.vcq_num = cq->gdma_id;
pkg.tx_oob.s_oob.vsq_frame = txq->vsq_frame;
@@ -442,8 +457,6 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
}
}
- WARN_ON_ONCE(pkg.wqe_req.num_sge > MAX_TX_WQE_SGL_ENTRIES);
-
if (pkg.wqe_req.num_sge <= ARRAY_SIZE(pkg.sgl_array)) {
pkg.wqe_req.sgl = pkg.sgl_array;
} else {
@@ -518,6 +531,25 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
+static netdev_features_t mana_features_check(struct sk_buff *skb,
+ struct net_device *ndev,
+ netdev_features_t features)
+{
+ if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
+ skb_shinfo(skb)->nr_frags + 2 > MAX_TX_WQE_SGL_ENTRIES) {
+ /* Exceeds HW SGE limit.
+ * GSO case:
+ * Disable GSO so the stack will software-segment the skb
+ * into smaller skbs that fit the SGE budget.
+ * Non-GSO case:
+ * The xmit path will attempt skb_linearize() as a fallback.
+ */
+ if (skb_is_gso(skb))
+ features &= ~NETIF_F_GSO_MASK;
+ }
+ return features;
+}
+
static void mana_get_stats64(struct net_device *ndev,
struct rtnl_link_stats64 *st)
{
@@ -878,6 +910,7 @@ static const struct net_device_ops mana_devops = {
.ndo_open = mana_open,
.ndo_stop = mana_close,
.ndo_select_queue = mana_select_queue,
+ .ndo_features_check = mana_features_check,
.ndo_start_xmit = mana_start_xmit,
.ndo_validate_addr = eth_validate_addr,
.ndo_get_stats64 = mana_get_stats64,
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index a1afa75a9463..fa5e1a2f06a9 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -71,6 +71,8 @@ static const struct mana_stats_desc mana_eth_stats[] = {
{"tx_cq_err", offsetof(struct mana_ethtool_stats, tx_cqe_err)},
{"tx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
tx_cqe_unknown_type)},
+ {"linear_pkt_tx_cnt", offsetof(struct mana_ethtool_stats,
+ linear_pkt_tx_cnt)},
{"rx_coalesced_err", offsetof(struct mana_ethtool_stats,
rx_coalesced_err)},
{"rx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 637f42485dba..84614ebe0f4c 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -592,6 +592,9 @@ enum {
#define GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE BIT(17)
#define GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE BIT(6)
+/* Driver supports linearizing the skb when num_sge exceeds hardware limit */
+#define GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE BIT(20)
+
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
@@ -601,7 +604,8 @@ enum {
GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT | \
GDMA_DRV_CAP_FLAG_1_SELF_RESET_ON_EQE | \
GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE | \
- GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE)
+ GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE | \
+ GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE)
#define GDMA_DRV_CAP_FLAGS2 0
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 8906901535f5..50a532fb30d6 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -404,6 +404,7 @@ struct mana_ethtool_stats {
u64 hc_tx_err_gdma;
u64 tx_cqe_err;
u64 tx_cqe_unknown_type;
+ u64 linear_pkt_tx_cnt;
u64 rx_coalesced_err;
u64 rx_cqe_unknown_type;
};
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v4 2/2] net: mana: Drop TX skb on post_work_request failure and unmap resources
From: Aditya Garg @ 2025-11-12 13:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, ssengar,
ernis, dipayanroy, shirazsaleem, leon, mlevitsk, yury.norov,
sbhatta, linux-hyperv, netdev, linux-kernel, linux-rdma,
gargaditya
Cc: Aditya Garg
In-Reply-To: <1762952506-23593-1-git-send-email-gargaditya@linux.microsoft.com>
Drop TX packets when posting the work request fails and ensure DMA
mappings are always cleaned up.
Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
---
Changes in v4:
* Fix warning during build reported by kernel test robot
---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 6 +-----
drivers/net/ethernet/microsoft/mana/mana_en.c | 7 +++----
include/net/mana/mana.h | 1 +
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index effe0a2f207a..8fd70b34807a 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1300,7 +1300,6 @@ int mana_gd_post_work_request(struct gdma_queue *wq,
struct gdma_posted_wqe_info *wqe_info)
{
u32 client_oob_size = wqe_req->inline_oob_size;
- struct gdma_context *gc;
u32 sgl_data_size;
u32 max_wqe_size;
u32 wqe_size;
@@ -1330,11 +1329,8 @@ int mana_gd_post_work_request(struct gdma_queue *wq,
if (wqe_size > max_wqe_size)
return -EINVAL;
- if (wq->monitor_avl_buf && wqe_size > mana_gd_wq_avail_space(wq)) {
- gc = wq->gdma_dev->gdma_context;
- dev_err(gc->dev, "unsuccessful flow control!\n");
+ if (wq->monitor_avl_buf && wqe_size > mana_gd_wq_avail_space(wq))
return -ENOSPC;
- }
if (wqe_info)
wqe_info->wqe_size_in_bu = wqe_size / GDMA_WQE_BU_SIZE;
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 67ae5421f9ee..066d822f68f0 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -491,9 +491,9 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
if (err) {
(void)skb_dequeue_tail(&txq->pending_skbs);
+ mana_unmap_skb(skb, apc);
netdev_warn(ndev, "Failed to post TX OOB: %d\n", err);
- err = NETDEV_TX_BUSY;
- goto tx_busy;
+ goto free_sgl_ptr;
}
err = NETDEV_TX_OK;
@@ -513,7 +513,6 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
tx_stats->bytes += len + ((num_gso_seg - 1) * gso_hs);
u64_stats_update_end(&tx_stats->syncp);
-tx_busy:
if (netif_tx_queue_stopped(net_txq) && mana_can_tx(gdma_sq)) {
netif_tx_wake_queue(net_txq);
apc->eth_stats.wake_queue++;
@@ -1679,7 +1678,7 @@ static int mana_move_wq_tail(struct gdma_queue *wq, u32 num_units)
return 0;
}
-static void mana_unmap_skb(struct sk_buff *skb, struct mana_port_context *apc)
+void mana_unmap_skb(struct sk_buff *skb, struct mana_port_context *apc)
{
struct mana_skb_head *ash = (struct mana_skb_head *)skb->head;
struct gdma_context *gc = apc->ac->gdma_dev->gdma_context;
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 50a532fb30d6..d05457d3e1ab 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -585,6 +585,7 @@ int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed,
void mana_query_phy_stats(struct mana_port_context *apc);
int mana_pre_alloc_rxbufs(struct mana_port_context *apc, int mtu, int num_queues);
void mana_pre_dealloc_rxbufs(struct mana_port_context *apc);
+void mana_unmap_skb(struct sk_buff *skb, struct mana_port_context *apc);
extern const struct ethtool_ops mana_ethtool_ops;
extern struct dentry *mana_debugfs_root;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v4 1/2] net: mana: Handle SKB if TX SGEs exceed hardware limit
From: Eric Dumazet @ 2025-11-12 13:55 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, kuba, pabeni,
longli, kotaranov, horms, shradhagupta, ssengar, ernis,
dipayanroy, shirazsaleem, leon, mlevitsk, yury.norov, sbhatta,
linux-hyperv, netdev, linux-kernel, linux-rdma, gargaditya
In-Reply-To: <1762952506-23593-2-git-send-email-gargaditya@linux.microsoft.com>
On Wed, Nov 12, 2025 at 5:11 AM Aditya Garg
<gargaditya@linux.microsoft.com> wrote:
>
> The MANA hardware supports a maximum of 30 scatter-gather entries (SGEs)
> per TX WQE. Exceeding this limit can cause TX failures.
> Add ndo_features_check() callback to validate SKB layout before
> transmission. For GSO SKBs that would exceed the hardware SGE limit, clear
> NETIF_F_GSO_MASK to enforce software segmentation in the stack.
> Add a fallback in mana_start_xmit() to linearize non-GSO SKBs that still
> exceed the SGE limit.
>
> Also, Add ethtool counter for SKBs linearized
>
> Co-developed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 37 ++++++++++++++++++-
> .../ethernet/microsoft/mana/mana_ethtool.c | 2 +
> include/net/mana/gdma.h | 6 ++-
> include/net/mana/mana.h | 1 +
> 4 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index cccd5b63cee6..67ae5421f9ee 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -11,6 +11,7 @@
> #include <linux/mm.h>
> #include <linux/pci.h>
> #include <linux/export.h>
> +#include <linux/skbuff.h>
>
> #include <net/checksum.h>
> #include <net/ip6_checksum.h>
> @@ -329,6 +330,20 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> cq = &apc->tx_qp[txq_idx].tx_cq;
> tx_stats = &txq->stats;
>
> + if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
> + skb_shinfo(skb)->nr_frags + 2 > MAX_TX_WQE_SGL_ENTRIES) {
> + /* GSO skb with Hardware SGE limit exceeded is not expected here
> + * as they are handled in mana_features_check() callback
> + */
> + if (skb_linearize(skb)) {
> + netdev_warn_once(ndev, "Failed to linearize skb with nr_frags=%d and is_gso=%d\n",
> + skb_shinfo(skb)->nr_frags,
> + skb_is_gso(skb));
> + goto tx_drop_count;
> + }
> + apc->eth_stats.linear_pkt_tx_cnt++;
> + }
> +
> pkg.tx_oob.s_oob.vcq_num = cq->gdma_id;
> pkg.tx_oob.s_oob.vsq_frame = txq->vsq_frame;
>
> @@ -442,8 +457,6 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> }
> }
>
> - WARN_ON_ONCE(pkg.wqe_req.num_sge > MAX_TX_WQE_SGL_ENTRIES);
> -
> if (pkg.wqe_req.num_sge <= ARRAY_SIZE(pkg.sgl_array)) {
> pkg.wqe_req.sgl = pkg.sgl_array;
> } else {
> @@ -518,6 +531,25 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> return NETDEV_TX_OK;
> }
>
#if MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES
> +static netdev_features_t mana_features_check(struct sk_buff *skb,
> + struct net_device *ndev,
> + netdev_features_t features)
> +{
> + if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
> + skb_shinfo(skb)->nr_frags + 2 > MAX_TX_WQE_SGL_ENTRIES) {
> + /* Exceeds HW SGE limit.
> + * GSO case:
> + * Disable GSO so the stack will software-segment the skb
> + * into smaller skbs that fit the SGE budget.
> + * Non-GSO case:
> + * The xmit path will attempt skb_linearize() as a fallback.
> + */
> + if (skb_is_gso(skb))
No need to test skb_is_gso(skb), you can clear bits, this will be a
NOP if the packet is non GSO anyway.
> + features &= ~NETIF_F_GSO_MASK;
> + }
> + return features;
> +}
#endif
> +
> static void mana_get_stats64(struct net_device *ndev,
> struct rtnl_link_stats64 *st)
> {
> @@ -878,6 +910,7 @@ static const struct net_device_ops mana_devops = {
> .ndo_open = mana_open,
> .ndo_stop = mana_close,
> .ndo_select_queue = mana_select_queue,
> + .ndo_features_check = mana_features_check,
Note that if your mana_features_check() is a nop if MAX_SKB_FRAGS is
small enough,
you could set a non NULL .ndo_features_check based on a preprocessor condition
#if MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES
.ndo_features_check = ....
#endif
This would avoid an expensive indirect call when possible.
> .ndo_start_xmit = mana_start_xmit,
> .ndo_validate_addr = eth_validate_addr,
> .ndo_get_stats64 = mana_get_stats64,
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> index a1afa75a9463..fa5e1a2f06a9 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> @@ -71,6 +71,8 @@ static const struct mana_stats_desc mana_eth_stats[] = {
> {"tx_cq_err", offsetof(struct mana_ethtool_stats, tx_cqe_err)},
> {"tx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
> tx_cqe_unknown_type)},
> + {"linear_pkt_tx_cnt", offsetof(struct mana_ethtool_stats,
> + linear_pkt_tx_cnt)},
> {"rx_coalesced_err", offsetof(struct mana_ethtool_stats,
> rx_coalesced_err)},
> {"rx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index 637f42485dba..84614ebe0f4c 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -592,6 +592,9 @@ enum {
> #define GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE BIT(17)
> #define GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE BIT(6)
>
> +/* Driver supports linearizing the skb when num_sge exceeds hardware limit */
> +#define GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE BIT(20)
> +
> #define GDMA_DRV_CAP_FLAGS1 \
> (GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
> GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
> @@ -601,7 +604,8 @@ enum {
> GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT | \
> GDMA_DRV_CAP_FLAG_1_SELF_RESET_ON_EQE | \
> GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE | \
> - GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE)
> + GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE | \
> + GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE)
>
> #define GDMA_DRV_CAP_FLAGS2 0
>
> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index 8906901535f5..50a532fb30d6 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h
> @@ -404,6 +404,7 @@ struct mana_ethtool_stats {
> u64 hc_tx_err_gdma;
> u64 tx_cqe_err;
> u64 tx_cqe_unknown_type;
> + u64 linear_pkt_tx_cnt;
> u64 rx_coalesced_err;
> u64 rx_cqe_unknown_type;
> };
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH net-next v9 01/14] vsock: a per-net vsock NS mode state
From: Stefano Garzarella @ 2025-11-12 14:13 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-1-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:43PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add the per-net vsock NS mode state. This only adds the structure for
>holding the mode and some of the functions for setting/getting and
>checking the mode, but does not integrate the functionality yet.
>
>A "net_mode" field is added to vsock_sock to store the mode of the
>namespace when the vsock_sock was created. In order to evaluate
>namespace mode rules we need to know both a) which namespace the
>endpoints are in, and b) what mode that namespace had when the endpoints
>were created. This allows us to handle the changing of modes from global
>to local *after* a socket has been created by remembering that the mode
>was global when the socket was created. If we were to use the current
>net's mode instead, then the lookup would fail and the socket would
>break.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- use xchg(), WRITE_ONCE(), READ_ONCE() for mode and mode_locked (Stefano)
>- clarify mode0/mode1 meaning in vsock_net_check_mode() comment
>- remove spin lock in net->vsock (not used anymore)
>- change mode from u8 to enum vsock_net_mode in vsock_net_write_mode()
>
>Changes in v7:
>- clarify vsock_net_check_mode() comments
>- change to `orig_net_mode == VSOCK_NET_MODE_GLOBAL && orig_net_mode == vsk->orig_net_mode`
>- remove extraneous explanation of `orig_net_mode`
>- rename `written` to `mode_locked`
>- rename `vsock_hdr` to `sysctl_hdr`
>- change `orig_net_mode` to `net_mode`
>- make vsock_net_check_mode() more generic by taking just net pointers
> and modes, instead of a vsock_sock ptr, for reuse by transports
> (e.g., vhost_vsock)
>
>Changes in v6:
>- add orig_net_mode to store mode at creation time which will be used to
> avoid breakage when namespace changes mode during socket/VM lifespan
>
>Changes in v5:
>- use /proc/sys/net/vsock/ns_mode instead of /proc/net/vsock_ns_mode
>- change from net->vsock.ns_mode to net->vsock.mode
>- change vsock_net_set_mode() to vsock_net_write_mode()
>- vsock_net_write_mode() returns bool for write success to avoid
> need to use vsock_net_mode_can_set()
>- remove vsock_net_mode_can_set()
>---
> MAINTAINERS | 1 +
> include/net/af_vsock.h | 41 +++++++++++++++++++++++++++++++++++++++++
> include/net/net_namespace.h | 4 ++++
> include/net/netns/vsock.h | 17 +++++++++++++++++
> 4 files changed, 63 insertions(+)
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 0dc4aa37d903..15c590a571f2 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -27098,6 +27098,7 @@ L: netdev@vger.kernel.org
> S: Maintained
> F: drivers/vhost/vsock.c
> F: include/linux/virtio_vsock.h
>+F: include/net/netns/vsock.h
> F: include/uapi/linux/virtio_vsock.h
> F: net/vmw_vsock/virtio_transport.c
> F: net/vmw_vsock/virtio_transport_common.c
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index d40e978126e3..f3c3f74355e8 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -10,6 +10,7 @@
>
> #include <linux/kernel.h>
> #include <linux/workqueue.h>
>+#include <net/netns/vsock.h>
> #include <net/sock.h>
> #include <uapi/linux/vm_sockets.h>
>
>@@ -65,6 +66,7 @@ struct vsock_sock {
> u32 peer_shutdown;
> bool sent_request;
> bool ignore_connecting_rst;
>+ enum vsock_net_mode net_mode;
>
> /* Protected by lock_sock(sk) */
> u64 buffer_size;
>@@ -256,4 +258,43 @@ static inline bool vsock_msgzerocopy_allow(const struct vsock_transport *t)
> {
> return t->msgzerocopy_allow && t->msgzerocopy_allow();
> }
>+
>+static inline enum vsock_net_mode vsock_net_mode(struct net *net)
>+{
>+ return READ_ONCE(net->vsock.mode);
>+}
>+
>+static inline bool vsock_net_write_mode(struct net *net, enum vsock_net_mode mode)
>+{
>+ if (xchg(&net->vsock.mode_locked, true))
LGTM, but it seems that some architecture doesn't support xchg on 1
byte, e.g. see commit d66a65b7f5d2 ("scsi: elx: efct: Fix link error for
_bad_cmpxchg")
So maybe we just need to change the type of mode_locked to int.
The rest LGTM.
Stefano
>+ return false;
>+
>+ WRITE_ONCE(net->vsock.mode, mode);
>+ return true;
>+}
>+
>+/* Return true if two namespaces and modes pass the mode rules. Otherwise,
>+ * return false.
>+ *
>+ * - ns0 and ns1 are the namespaces being checked.
>+ * - mode0 and mode1 are the vsock namespace modes of ns0 and ns1 at the time
>+ * the vsock objects were created.
>+ *
>+ * Read more about modes in the comment header of net/vmw_vsock/af_vsock.c.
>+ */
>+static inline bool vsock_net_check_mode(struct net *ns0, enum vsock_net_mode mode0,
>+ struct net *ns1, enum vsock_net_mode mode1)
>+{
>+ /* Any vsocks within the same network namespace are always reachable,
>+ * regardless of the mode.
>+ */
>+ if (net_eq(ns0, ns1))
>+ return true;
>+
>+ /*
>+ * If the network namespaces differ, vsocks are only reachable if both
>+ * were created in VSOCK_NET_MODE_GLOBAL mode.
>+ */
>+ return mode0 == VSOCK_NET_MODE_GLOBAL && mode0 == mode1;
>+}
> #endif /* __AF_VSOCK_H__ */
>diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
>index cb664f6e3558..66d3de1d935f 100644
>--- a/include/net/net_namespace.h
>+++ b/include/net/net_namespace.h
>@@ -37,6 +37,7 @@
> #include <net/netns/smc.h>
> #include <net/netns/bpf.h>
> #include <net/netns/mctp.h>
>+#include <net/netns/vsock.h>
> #include <net/net_trackers.h>
> #include <linux/ns_common.h>
> #include <linux/idr.h>
>@@ -196,6 +197,9 @@ struct net {
> /* Move to a better place when the config guard is removed. */
> struct mutex rtnl_mutex;
> #endif
>+#if IS_ENABLED(CONFIG_VSOCKETS)
>+ struct netns_vsock vsock;
>+#endif
> } __randomize_layout;
>
> #include <linux/seq_file_net.h>
>diff --git a/include/net/netns/vsock.h b/include/net/netns/vsock.h
>new file mode 100644
>index 000000000000..21189d7bdd4e
>--- /dev/null
>+++ b/include/net/netns/vsock.h
>@@ -0,0 +1,17 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+#ifndef __NET_NET_NAMESPACE_VSOCK_H
>+#define __NET_NET_NAMESPACE_VSOCK_H
>+
>+#include <linux/types.h>
>+
>+enum vsock_net_mode {
>+ VSOCK_NET_MODE_GLOBAL,
>+ VSOCK_NET_MODE_LOCAL,
>+};
>+
>+struct netns_vsock {
>+ struct ctl_table_header *sysctl_hdr;
>+ enum vsock_net_mode mode;
>+ bool mode_locked;
>+};
>+#endif /* __NET_NET_NAMESPACE_VSOCK_H */
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 02/14] vsock: add netns to vsock core
From: Stefano Garzarella @ 2025-11-12 14:14 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-2-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:44PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add netns logic to vsock core. Additionally, modify transport hook
>prototypes to be used by later transport-specific patches (e.g.,
>*_seqpacket_allow()).
>
>Namespaces are supported primarily by changing socket lookup functions
>(e.g., vsock_find_connected_socket()) to take into account the socket
>namespace and the namespace mode before considering a candidate socket a
>"match".
>
>This patch also introduces the sysctl /proc/sys/net/vsock/ns_mode that
>accepts the "global" or "local" mode strings.
>
>Add netns functionality (initialization, passing to transports, procfs,
>etc...) to the af_vsock socket layer. Later patches that add netns
>support to transports depend on this patch.
>
>seqpacket_allow() callbacks are modified to take a vsk so that transport
>implementations can inspect sock_net(sk) and vsk->net_mode when performing
>lookups (e.g., vhost does this in its future netns patch). Because the
>API change affects all transports, it seemed more appropriate to make
>this internal API change in the "vsock core" patch then in the "vhost"
>patch.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- remove virtio_vsock_alloc_rx_skb() (Stefano)
>- remove vsock_global_dummy_net, not needed as net=NULL +
> net_mode=VSOCK_NET_MODE_GLOBAL achieves identical result
>
>Changes in v7:
>- hv_sock: fix hyperv build error
>- explain why vhost does not use the dummy
>- explain usage of __vsock_global_dummy_net
>- explain why VSOCK_NET_MODE_STR_MAX is 8 characters
>- use switch-case in vsock_net_mode_string()
>- avoid changing transports as much as possible
>- add vsock_find_{bound,connected}_socket_net()
>- rename `vsock_hdr` to `sysctl_hdr`
>- add virtio_vsock_alloc_linear_skb() wrapper for setting dummy net and
> global mode for virtio-vsock, move skb->cb zero-ing into wrapper
>- explain seqpacket_allow() change
>- move net setting to __vsock_create() instead of vsock_create() so
> that child sockets also have their net assigned upon accept()
>
>Changes in v6:
>- unregister sysctl ops in vsock_exit()
>- af_vsock: clarify description of CID behavior
>- af_vsock: fix buf vs buffer naming, and length checking
>- af_vsock: fix length checking w/ correct ctl_table->maxlen
>
>Changes in v5:
>- vsock_global_net() -> vsock_global_dummy_net()
>- update comments for new uAPI
>- use /proc/sys/net/vsock/ns_mode instead of /proc/net/vsock_ns_mode
>- add prototype changes so patch remains compilable
>---
> drivers/vhost/vsock.c | 4 +-
> include/net/af_vsock.h | 8 +-
> net/vmw_vsock/af_vsock.c | 251 ++++++++++++++++++++++++++++++++++++---
> net/vmw_vsock/virtio_transport.c | 4 +-
> net/vmw_vsock/vsock_loopback.c | 4 +-
> 5 files changed, 247 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index ae01457ea2cd..34adf0cf9124 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -404,7 +404,7 @@ static bool vhost_transport_msgzerocopy_allow(void)
> return true;
> }
>
>-static bool vhost_transport_seqpacket_allow(u32 remote_cid);
>+static bool vhost_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid);
>
> static struct virtio_transport vhost_transport = {
> .transport = {
>@@ -460,7 +460,7 @@ static struct virtio_transport vhost_transport = {
> .send_pkt = vhost_transport_send_pkt,
> };
>
>-static bool vhost_transport_seqpacket_allow(u32 remote_cid)
>+static bool vhost_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
> {
> struct vhost_vsock *vsock;
> bool seqpacket_allow = false;
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index f3c3f74355e8..cfd121bb5ab7 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -145,7 +145,7 @@ struct vsock_transport {
> int flags);
> int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg,
> size_t len);
>- bool (*seqpacket_allow)(u32 remote_cid);
>+ bool (*seqpacket_allow)(struct vsock_sock *vsk, u32 remote_cid);
> u32 (*seqpacket_has_data)(struct vsock_sock *vsk);
>
> /* Notification. */
>@@ -218,6 +218,12 @@ void vsock_remove_connected(struct vsock_sock *vsk);
> struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr);
> struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
> struct sockaddr_vm *dst);
>+struct sock *vsock_find_bound_socket_net(struct sockaddr_vm *addr, struct net *net,
>+ enum vsock_net_mode net_mode);
>+struct sock *vsock_find_connected_socket_net(struct sockaddr_vm *src,
>+ struct sockaddr_vm *dst,
>+ struct net *net,
>+ enum vsock_net_mode net_mode);
> void vsock_remove_sock(struct vsock_sock *vsk);
> void vsock_for_each_connected_socket(struct vsock_transport *transport,
> void (*fn)(struct sock *sk));
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 72bb6b7ed386..c0b5946bdc95 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -83,6 +83,35 @@
> * TCP_ESTABLISHED - connected
> * TCP_CLOSING - disconnecting
> * TCP_LISTEN - listening
>+ *
>+ * - Namespaces in vsock support two different modes configured
>+ * through /proc/sys/net/vsock/ns_mode. The modes are "local" and "global".
>+ * Each mode defines how the namespace interacts with CIDs.
>+ * /proc/sys/net/vsock/ns_mode is write-once, so that it may be configured
>+ * and locked down by a namespace manager. The default is "global". The mode
>+ * is set per-namespace.
>+ *
>+ * The modes affect the allocation and accessibility of CIDs as follows:
>+ *
>+ * - global - access and allocation are all system-wide
>+ * - all CID allocation from global namespaces draw from the same
>+ * system-wide pool
>+ * - if one global namespace has already allocated some CID, another
>+ * global namespace will not be able to allocate the same CID
>+ * - global mode AF_VSOCK sockets can reach any VM or socket in any global
>+ * namespace, they are not contained to only their own namespace
>+ * - AF_VSOCK sockets in a global mode namespace cannot reach VMs or
>+ * sockets in any local mode namespace
>+ * - local - access and allocation are contained within the namespace
>+ * - CID allocation draws only from a private pool local only to the
>+ * namespace, and does not affect the CIDs available for allocation in any
>+ * other namespace (global or local)
>+ * - VMs in a local namespace do not collide with CIDs in any other local
>+ * namespace or any global namespace. For example, if a VM in a local mode
>+ * namespace is given CID 10, then CID 10 is still available for
>+ * allocation in any other namespace, but not in the same namespace
>+ * - AF_VSOCK sockets in a local mode namespace can connect only to VMs or
>+ * other sockets within their own namespace.
Should we also document what happen to pre-existing sockets/devices if
the user change the mode ?
The rest LGTM!
Thanks,
Stefano
> */
>
> #include <linux/compat.h>
>@@ -100,6 +129,7 @@
> #include <linux/module.h>
> #include <linux/mutex.h>
> #include <linux/net.h>
>+#include <linux/proc_fs.h>
> #include <linux/poll.h>
> #include <linux/random.h>
> #include <linux/skbuff.h>
>@@ -111,9 +141,18 @@
> #include <linux/workqueue.h>
> #include <net/sock.h>
> #include <net/af_vsock.h>
>+#include <net/netns/vsock.h>
> #include <uapi/linux/vm_sockets.h>
> #include <uapi/asm-generic/ioctls.h>
>
>+#define VSOCK_NET_MODE_STR_GLOBAL "global"
>+#define VSOCK_NET_MODE_STR_LOCAL "local"
>+
>+/* 6 chars for "global", 1 for null-terminator, and 1 more for '\n'.
>+ * The newline is added by proc_dostring() for read operations.
>+ */
>+#define VSOCK_NET_MODE_STR_MAX 8
>+
> static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr);
> static void vsock_sk_destruct(struct sock *sk);
> static int vsock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
>@@ -235,33 +274,44 @@ static void __vsock_remove_connected(struct vsock_sock *vsk)
> sock_put(&vsk->sk);
> }
>
>-static struct sock *__vsock_find_bound_socket(struct sockaddr_vm *addr)
>+static struct sock *__vsock_find_bound_socket_net(struct sockaddr_vm *addr,
>+ struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct vsock_sock *vsk;
>
> list_for_each_entry(vsk, vsock_bound_sockets(addr), bound_table) {
>- if (vsock_addr_equals_addr(addr, &vsk->local_addr))
>- return sk_vsock(vsk);
>+ struct sock *sk = sk_vsock(vsk);
>+
>+ if (vsock_addr_equals_addr(addr, &vsk->local_addr) &&
>+ vsock_net_check_mode(sock_net(sk), vsk->net_mode, net, net_mode))
>+ return sk;
>
> if (addr->svm_port == vsk->local_addr.svm_port &&
> (vsk->local_addr.svm_cid == VMADDR_CID_ANY ||
>- addr->svm_cid == VMADDR_CID_ANY))
>- return sk_vsock(vsk);
>+ addr->svm_cid == VMADDR_CID_ANY) &&
>+ vsock_net_check_mode(sock_net(sk), vsk->net_mode, net, net_mode))
>+ return sk;
> }
>
> return NULL;
> }
>
>-static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
>- struct sockaddr_vm *dst)
>+static struct sock *__vsock_find_connected_socket_net(struct sockaddr_vm *src,
>+ struct sockaddr_vm *dst,
>+ struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct vsock_sock *vsk;
>
> list_for_each_entry(vsk, vsock_connected_sockets(src, dst),
> connected_table) {
>+ struct sock *sk = sk_vsock(vsk);
>+
> if (vsock_addr_equals_addr(src, &vsk->remote_addr) &&
>- dst->svm_port == vsk->local_addr.svm_port) {
>- return sk_vsock(vsk);
>+ dst->svm_port == vsk->local_addr.svm_port &&
>+ vsock_net_check_mode(sock_net(sk), vsk->net_mode, net, net_mode)) {
>+ return sk;
> }
> }
>
>@@ -304,12 +354,14 @@ void vsock_remove_connected(struct vsock_sock *vsk)
> }
> EXPORT_SYMBOL_GPL(vsock_remove_connected);
>
>-struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr)
>+struct sock *vsock_find_bound_socket_net(struct sockaddr_vm *addr,
>+ struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct sock *sk;
>
> spin_lock_bh(&vsock_table_lock);
>- sk = __vsock_find_bound_socket(addr);
>+ sk = __vsock_find_bound_socket_net(addr, net, net_mode);
> if (sk)
> sock_hold(sk);
>
>@@ -317,15 +369,23 @@ struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr)
>
> return sk;
> }
>+EXPORT_SYMBOL_GPL(vsock_find_bound_socket_net);
>+
>+struct sock *vsock_find_bound_socket(struct sockaddr_vm *addr)
>+{
>+ return vsock_find_bound_socket_net(addr, NULL, VSOCK_NET_MODE_GLOBAL);
>+}
> EXPORT_SYMBOL_GPL(vsock_find_bound_socket);
>
>-struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
>- struct sockaddr_vm *dst)
>+struct sock *vsock_find_connected_socket_net(struct sockaddr_vm *src,
>+ struct sockaddr_vm *dst,
>+ struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct sock *sk;
>
> spin_lock_bh(&vsock_table_lock);
>- sk = __vsock_find_connected_socket(src, dst);
>+ sk = __vsock_find_connected_socket_net(src, dst, net, net_mode);
> if (sk)
> sock_hold(sk);
>
>@@ -333,6 +393,14 @@ struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
>
> return sk;
> }
>+EXPORT_SYMBOL_GPL(vsock_find_connected_socket_net);
>+
>+struct sock *vsock_find_connected_socket(struct sockaddr_vm *src,
>+ struct sockaddr_vm *dst)
>+{
>+ return vsock_find_connected_socket_net(src, dst,
>+ NULL, VSOCK_NET_MODE_GLOBAL);
>+}
> EXPORT_SYMBOL_GPL(vsock_find_connected_socket);
>
> void vsock_remove_sock(struct vsock_sock *vsk)
>@@ -528,7 +596,7 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>
> if (sk->sk_type == SOCK_SEQPACKET) {
> if (!new_transport->seqpacket_allow ||
>- !new_transport->seqpacket_allow(remote_cid)) {
>+ !new_transport->seqpacket_allow(vsk, remote_cid)) {
> module_put(new_transport->module);
> return -ESOCKTNOSUPPORT;
> }
>@@ -676,6 +744,7 @@ static void vsock_pending_work(struct work_struct *work)
> static int __vsock_bind_connectible(struct vsock_sock *vsk,
> struct sockaddr_vm *addr)
> {
>+ struct net *net = sock_net(sk_vsock(vsk));
> static u32 port;
> struct sockaddr_vm new_addr;
>
>@@ -695,7 +764,8 @@ static int __vsock_bind_connectible(struct vsock_sock *vsk,
>
> new_addr.svm_port = port++;
>
>- if (!__vsock_find_bound_socket(&new_addr)) {
>+ if (!__vsock_find_bound_socket_net(&new_addr, net,
>+ vsk->net_mode)) {
> found = true;
> break;
> }
>@@ -712,7 +782,8 @@ static int __vsock_bind_connectible(struct vsock_sock *vsk,
> return -EACCES;
> }
>
>- if (__vsock_find_bound_socket(&new_addr))
>+ if (__vsock_find_bound_socket_net(&new_addr, net,
>+ vsk->net_mode))
> return -EADDRINUSE;
> }
>
>@@ -836,6 +907,8 @@ static struct sock *__vsock_create(struct net *net,
> vsk->buffer_max_size = VSOCK_DEFAULT_BUFFER_MAX_SIZE;
> }
>
>+ vsk->net_mode = vsock_net_mode(net);
>+
> return sk;
> }
>
>@@ -2636,6 +2709,141 @@ static struct miscdevice vsock_device = {
> .fops = &vsock_device_ops,
> };
>
>+static int vsock_net_mode_string(const struct ctl_table *table, int write,
>+ void *buffer, size_t *lenp, loff_t *ppos)
>+{
>+ char data[VSOCK_NET_MODE_STR_MAX] = {0};
>+ enum vsock_net_mode mode;
>+ struct ctl_table tmp;
>+ struct net *net;
>+ int ret;
>+
>+ if (!table->data || !table->maxlen || !*lenp) {
>+ *lenp = 0;
>+ return 0;
>+ }
>+
>+ net = current->nsproxy->net_ns;
>+ tmp = *table;
>+ tmp.data = data;
>+
>+ if (!write) {
>+ const char *p;
>+
>+ mode = vsock_net_mode(net);
>+
>+ switch (mode) {
>+ case VSOCK_NET_MODE_GLOBAL:
>+ p = VSOCK_NET_MODE_STR_GLOBAL;
>+ break;
>+ case VSOCK_NET_MODE_LOCAL:
>+ p = VSOCK_NET_MODE_STR_LOCAL;
>+ break;
>+ default:
>+ WARN_ONCE(true, "netns has invalid vsock mode");
>+ *lenp = 0;
>+ return 0;
>+ }
>+
>+ strscpy(data, p, sizeof(data));
>+ tmp.maxlen = strlen(p);
>+ }
>+
>+ ret = proc_dostring(&tmp, write, buffer, lenp, ppos);
>+ if (ret)
>+ return ret;
>+
>+ if (write) {
>+ if (*lenp >= sizeof(data))
>+ return -EINVAL;
>+
>+ if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
>+ mode = VSOCK_NET_MODE_GLOBAL;
>+ else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
>+ mode = VSOCK_NET_MODE_LOCAL;
>+ else
>+ return -EINVAL;
>+
>+ if (!vsock_net_write_mode(net, mode))
>+ return -EPERM;
>+ }
>+
>+ return 0;
>+}
>+
>+static struct ctl_table vsock_table[] = {
>+ {
>+ .procname = "ns_mode",
>+ .data = &init_net.vsock.mode,
>+ .maxlen = VSOCK_NET_MODE_STR_MAX,
>+ .mode = 0644,
>+ .proc_handler = vsock_net_mode_string
>+ },
>+};
>+
>+static int __net_init vsock_sysctl_register(struct net *net)
>+{
>+ struct ctl_table *table;
>+
>+ if (net_eq(net, &init_net)) {
>+ table = vsock_table;
>+ } else {
>+ table = kmemdup(vsock_table, sizeof(vsock_table), GFP_KERNEL);
>+ if (!table)
>+ goto err_alloc;
>+
>+ table[0].data = &net->vsock.mode;
>+ }
>+
>+ net->vsock.sysctl_hdr = register_net_sysctl_sz(net, "net/vsock", table,
>+ ARRAY_SIZE(vsock_table));
>+ if (!net->vsock.sysctl_hdr)
>+ goto err_reg;
>+
>+ return 0;
>+
>+err_reg:
>+ if (!net_eq(net, &init_net))
>+ kfree(table);
>+err_alloc:
>+ return -ENOMEM;
>+}
>+
>+static void vsock_sysctl_unregister(struct net *net)
>+{
>+ const struct ctl_table *table;
>+
>+ table = net->vsock.sysctl_hdr->ctl_table_arg;
>+ unregister_net_sysctl_table(net->vsock.sysctl_hdr);
>+ if (!net_eq(net, &init_net))
>+ kfree(table);
>+}
>+
>+static void vsock_net_init(struct net *net)
>+{
>+ net->vsock.mode = VSOCK_NET_MODE_GLOBAL;
>+}
>+
>+static __net_init int vsock_sysctl_init_net(struct net *net)
>+{
>+ vsock_net_init(net);
>+
>+ if (vsock_sysctl_register(net))
>+ return -ENOMEM;
>+
>+ return 0;
>+}
>+
>+static __net_exit void vsock_sysctl_exit_net(struct net *net)
>+{
>+ vsock_sysctl_unregister(net);
>+}
>+
>+static struct pernet_operations vsock_sysctl_ops __net_initdata = {
>+ .init = vsock_sysctl_init_net,
>+ .exit = vsock_sysctl_exit_net,
>+};
>+
> static int __init vsock_init(void)
> {
> int err = 0;
>@@ -2663,10 +2871,18 @@ static int __init vsock_init(void)
> goto err_unregister_proto;
> }
>
>+ if (register_pernet_subsys(&vsock_sysctl_ops)) {
>+ err = -ENOMEM;
>+ goto err_unregister_sock;
>+ }
>+
>+ vsock_net_init(&init_net);
> vsock_bpf_build_proto();
>
> return 0;
>
>+err_unregister_sock:
>+ sock_unregister(AF_VSOCK);
> err_unregister_proto:
> proto_unregister(&vsock_proto);
> err_deregister_misc:
>@@ -2680,6 +2896,7 @@ static void __exit vsock_exit(void)
> misc_deregister(&vsock_device);
> sock_unregister(AF_VSOCK);
> proto_unregister(&vsock_proto);
>+ unregister_pernet_subsys(&vsock_sysctl_ops);
> }
>
> const struct vsock_transport *vsock_core_get_transport(struct vsock_sock *vsk)
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index 8c867023a2e5..f92f23be3f59 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -536,7 +536,7 @@ static bool virtio_transport_msgzerocopy_allow(void)
> return true;
> }
>
>-static bool virtio_transport_seqpacket_allow(u32 remote_cid);
>+static bool virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid);
>
> static struct virtio_transport virtio_transport = {
> .transport = {
>@@ -593,7 +593,7 @@ static struct virtio_transport virtio_transport = {
> .can_msgzerocopy = virtio_transport_can_msgzerocopy,
> };
>
>-static bool virtio_transport_seqpacket_allow(u32 remote_cid)
>+static bool virtio_transport_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
> {
> struct virtio_vsock *vsock;
> bool seqpacket_allow;
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index bc2ff918b315..a8f218f0c5a3 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -46,7 +46,7 @@ static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
> return 0;
> }
>
>-static bool vsock_loopback_seqpacket_allow(u32 remote_cid);
>+static bool vsock_loopback_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid);
> static bool vsock_loopback_msgzerocopy_allow(void)
> {
> return true;
>@@ -106,7 +106,7 @@ static struct virtio_transport loopback_transport = {
> .send_pkt = vsock_loopback_send_pkt,
> };
>
>-static bool vsock_loopback_seqpacket_allow(u32 remote_cid)
>+static bool vsock_loopback_seqpacket_allow(struct vsock_sock *vsk, u32 remote_cid)
> {
> return true;
> }
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 03/14] vsock/virtio: add netns support to virtio transport and virtio common
From: Stefano Garzarella @ 2025-11-12 14:18 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-3-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:45PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Enable network namespace support in the virtio-vsock and common
>transport layer.
>
>The changes include:
This list seems to have been generated by AI. I have nothing against it,
but I don't think it's important to list all the things that have
changed, but rather to explain why.
>1. Add a 'net' field to virtio_vsock_pkt_info to carry the namespace
> pointer for outgoing packets.
Why?
>2. Add 'net' and 'net_mode' to t->send_pkt() and
> virtio_transport_recv_pkt() functions
Why?
>3. Modify callback functions to accept placeholder values
> (NULL and 0) for net and net_mode. The placeholders will be
Why 0 ? I mean VSOCK_NET_MODE_GLOBAL is also 0, no?
So I don't understand if you want to specify an invalid value (like
NULL) or VSOCK_NET_MODE_GLOBAL.
> replaced when later patches in this series add namespace support
> to transports.
>4. Set virtio-vsock to global mode unconditionally, instead of using
> placeholders. This is done in this patch because virtio-vsock won't
> have any additional changes to choose the net/net_mode, unlike the
> other transports. Same complexity as placeholders.
>5. Pass net and net_mode to virtio_transport_reset_no_sock() directly.
> This ensures that the outgoing RST packets are scoped based on the
> namespace of the receiver of the failed request.
"Receiver" is confusing IMO, see the comment on
virtio_transport_reset_no_sock().
>6. Pass net and net_mode to socket lookup functions using
> vsock_find_{bound,connected}_socket_net().
mmmm, are those functions working fine with the placeholders?
If it simplifies, I think we can eventually merge all changes to
transports that depends on virtio_transport_common in a single commit.
IMO is better to have working commits than better split.
I mean, is this commit working (at runtime) well?
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- include/virtio_vsock.h: send_pkt() cb takes net and net_mode
>- virtio_transport reset_no_sock() takes net and net_mode
>- vhost-vsock: add placeholders to recv_pkt() for compilation
>- loopback: add placeholders to recv_pkt() for compilation
>- remove skb->cb net/net_mode usage, pass as arguments to
> t->send_pkt() and virtio_transport_recv_pkt() functions instead.
> Note that skb->cb will still be used by loopback, but only internal
> to loopback and never passing it to virtio common.
>- remove virtio_vsock_alloc_rx_skb(), it is not needed after removing
> skb->cb usage.
>- pass net and net_mode to virtio_transport_reset_no_sock()
>
>Changes in v8:
>- add the virtio_vsock_alloc_rx_skb(), to be in same patch that fields
>are read (Stefano)
>
>Changes in v7:
>- add comment explaining the !vsk case in virtio_transport_alloc_skb()
>---
> drivers/vhost/vsock.c | 6 ++--
> include/linux/virtio_vsock.h | 8 +++--
> net/vmw_vsock/virtio_transport.c | 10 ++++--
> net/vmw_vsock/virtio_transport_common.c | 57 ++++++++++++++++++++++++---------
> net/vmw_vsock/vsock_loopback.c | 5 +--
> 5 files changed, 62 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 34adf0cf9124..0a0e73405532 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -269,7 +269,8 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
> }
>
> static int
>-vhost_transport_send_pkt(struct sk_buff *skb)
>+vhost_transport_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vhost_vsock *vsock;
>@@ -537,7 +538,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
> if (le64_to_cpu(hdr->src_cid) == vsock->guest_cid &&
> le64_to_cpu(hdr->dst_cid) ==
> vhost_transport_get_local_cid())
>- virtio_transport_recv_pkt(&vhost_transport, skb);
>+ virtio_transport_recv_pkt(&vhost_transport, skb, NULL,
>+ 0);
> else
> kfree_skb(skb);
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 0c67543a45c8..5ed6136a4ed4 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -173,6 +173,8 @@ struct virtio_vsock_pkt_info {
> u32 remote_cid, remote_port;
> struct vsock_sock *vsk;
> struct msghdr *msg;
>+ struct net *net;
>+ enum vsock_net_mode net_mode;
> u32 pkt_len;
> u16 type;
> u16 op;
>@@ -185,7 +187,8 @@ struct virtio_transport {
> struct vsock_transport transport;
>
> /* Takes ownership of the packet */
>- int (*send_pkt)(struct sk_buff *skb);
>+ int (*send_pkt)(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode);
>
> /* Used in MSG_ZEROCOPY mode. Checks, that provided data
> * (number of buffers) could be transmitted with zerocopy
>@@ -280,7 +283,8 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
> void virtio_transport_destruct(struct vsock_sock *vsk);
>
> void virtio_transport_recv_pkt(struct virtio_transport *t,
>- struct sk_buff *skb);
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode);
> void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index f92f23be3f59..9395fd875823 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -231,7 +231,8 @@ static int virtio_transport_send_skb_fast_path(struct virtio_vsock *vsock, struc
> }
>
> static int
>-virtio_transport_send_pkt(struct sk_buff *skb)
>+virtio_transport_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr;
> struct virtio_vsock *vsock;
>@@ -660,7 +661,12 @@ static void virtio_transport_rx_work(struct work_struct *work)
> virtio_vsock_skb_put(skb, payload_len);
>
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&virtio_transport, skb);
>+
>+ /* Force virtio-transport into global mode since it
>+ * does not yet support local-mode namespacing.
>+ */
>+ virtio_transport_recv_pkt(&virtio_transport, skb,
>+ NULL, VSOCK_NET_MODE_GLOBAL);
> }
> } while (!virtqueue_enable_cb(vq));
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index dcc8a1d5851e..f4e09cb1567c 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -413,7 +413,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>
> virtio_transport_inc_tx_pkt(vvs, skb);
>
>- ret = t_ops->send_pkt(skb);
>+ ret = t_ops->send_pkt(skb, info->net, info->net_mode);
> if (ret < 0)
> break;
>
>@@ -527,6 +527,8 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1067,6 +1069,8 @@ int virtio_transport_connect(struct vsock_sock *vsk)
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_REQUEST,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1082,6 +1086,8 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
> (mode & SEND_SHUTDOWN ?
> VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1108,6 +1114,8 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
> .msg = msg,
> .pkt_len = len,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1145,6 +1153,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> .op = VIRTIO_VSOCK_OP_RST,
> .reply = !!skb,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> /* Send RST only if the original pkt is not a RST pkt */
>@@ -1156,15 +1166,27 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
>
> /* Normally packets are associated with a socket. There may be no socket if an
> * attempt was made to connect to a socket that does not exist.
>+ *
>+ * net and net_mode refer to the net and mode of the receiving device (e.g.,
>+ * vhost_vsock). For loopback, they refer to the sending socket net/mode. This
>+ * way the RST packet is sent back to the same namespace as the bad request.
Could this be a problem, should we split this function?
BTW, I'm a bit confused. For vhost-vsock, this is the namespace of the
device, so the namespace of the guest, so also in that case the
namespace of the sender, no?
Maybe sender/receiver are confusing. What you want to highlight with
this comment?
> */
> static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>- struct sk_buff *skb)
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_RST,
> .type = le16_to_cpu(hdr->type),
> .reply = true,
>+
>+ /* net or net_mode are not defined here because we pass
>+ * net and net_mode directly to t->send_pkt(), instead of
>+ * relying on virtio_transport_send_pkt_info() to pass them to
>+ * t->send_pkt(). They are not needed by
>+ * virtio_transport_alloc_skb().
>+ */
> };
> struct sk_buff *reply;
>
>@@ -1183,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> if (!reply)
> return -ENOMEM;
>
>- return t->send_pkt(reply);
>+ return t->send_pkt(reply, net, net_mode);
> }
>
> /* This function should be called with sk_lock held and SOCK_DONE set */
>@@ -1465,6 +1487,8 @@ virtio_transport_send_response(struct vsock_sock *vsk,
> .remote_port = le32_to_cpu(hdr->src_port),
> .reply = true,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1507,12 +1531,12 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> int ret;
>
> if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_REQUEST) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> return -EINVAL;
> }
>
> if (sk_acceptq_is_full(sk)) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> return -ENOMEM;
> }
>
>@@ -1520,13 +1544,13 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> * Subsequent enqueues would lead to a memory leak.
> */
> if (sk->sk_shutdown == SHUTDOWN_MASK) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> return -ESHUTDOWN;
> }
>
> child = vsock_create_connected(sk);
> if (!child) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> return -ENOMEM;
> }
>
>@@ -1548,7 +1572,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> */
> if (ret || vchild->transport != &t->transport) {
> release_sock(child);
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> sock_put(child);
> return ret;
> }
>@@ -1576,7 +1600,8 @@ static bool virtio_transport_valid_type(u16 type)
> * lock.
> */
> void virtio_transport_recv_pkt(struct virtio_transport *t,
>- struct sk_buff *skb)
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct sockaddr_vm src, dst;
>@@ -1599,24 +1624,24 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> le32_to_cpu(hdr->fwd_cnt));
>
> if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> goto free_pkt;
> }
>
> /* The socket must be in connected or bound table
> * otherwise send reset back
> */
>- sk = vsock_find_connected_socket(&src, &dst);
>+ sk = vsock_find_connected_socket_net(&src, &dst, net, net_mode);
Here `net` can be null, right? Is this okay?
> if (!sk) {
>- sk = vsock_find_bound_socket(&dst);
>+ sk = vsock_find_bound_socket_net(&dst, net, net_mode);
> if (!sk) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> goto free_pkt;
> }
> }
>
> if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> sock_put(sk);
> goto free_pkt;
> }
>@@ -1635,7 +1660,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> */
> if (sock_flag(sk, SOCK_DONE) ||
> (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> release_sock(sk);
> sock_put(sk);
> goto free_pkt;
>@@ -1667,7 +1692,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> kfree_skb(skb);
> break;
> default:
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> kfree_skb(skb);
> break;
> }
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index a8f218f0c5a3..d3ac056663ea 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -26,7 +26,8 @@ static u32 vsock_loopback_get_local_cid(void)
> return VMADDR_CID_LOCAL;
> }
>
>-static int vsock_loopback_send_pkt(struct sk_buff *skb)
>+static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct vsock_loopback *vsock = &the_vsock_loopback;
> int len = skb->len;
>@@ -130,7 +131,7 @@ static void vsock_loopback_work(struct work_struct *work)
> */
> virtio_transport_consume_skb_sent(skb, false);
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&loopback_transport, skb);
>+ virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
> }
> }
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 06/14] vsock/loopback: add netns support
From: Stefano Garzarella @ 2025-11-12 14:19 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-6-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add NS support to vsock loopback. Sockets in a global mode netns
>communicate with each other, regardless of namespace. Sockets in a local
>mode netns may only communicate with other sockets within the same
>namespace.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- remove per-netns vsock_loopback and workqueues, just re-using
> the net and net_mode in skb->cb achieved the same result in a simpler
> way. Also removed need for pernet_subsys.
>- properly track net references
>
>Changes in v7:
>- drop for_each_net() init/exit, drop net_rwsem, the pernet registration
> handles this automatically and race-free
>- flush workqueue before destruction, purge pkt list
>- remember net_mode instead of current net mode
>- keep space after INIT_WORK()
>- change vsock_loopback in netns_vsock to ->priv void ptr
>- rename `orig_net_mode` to `net_mode`
>- remove useless comment
>- protect `register_pernet_subsys()` with `net_rwsem`
>- do cleanup before releasing `net_rwsem` when failure happens
>- call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
>- call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`
>
>Changes in v6:
>- init pernet ops for vsock_loopback module
>- vsock_loopback: add space in struct to clarify lock protection
>- do proper cleanup/unregister on vsock_loopback_exit()
>- vsock_loopback: use virtio_vsock_skb_net()
>
>Changes in v5:
>- add callbacks code to avoid reverse dependency
>- add logic for handling vsock_loopback setup for already existing
> namespaces
>---
> net/vmw_vsock/vsock_loopback.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index d3ac056663ea..e62f6c516992 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -32,6 +32,9 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
> struct vsock_loopback *vsock = &the_vsock_loopback;
> int len = skb->len;
>
>+ virtio_vsock_skb_set_net(skb, net);
>+ virtio_vsock_skb_set_net_mode(skb, net_mode);
>+
> virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
> queue_work(vsock->workqueue, &vsock->pkt_work);
>
>@@ -116,8 +119,10 @@ static void vsock_loopback_work(struct work_struct *work)
> {
> struct vsock_loopback *vsock =
> container_of(work, struct vsock_loopback, pkt_work);
>+ enum vsock_net_mode net_mode;
> struct sk_buff_head pkts;
> struct sk_buff *skb;
>+ struct net *net;
>
> skb_queue_head_init(&pkts);
>
>@@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
> */
> virtio_transport_consume_skb_sent(skb, false);
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
>+
>+ /* In the case of virtio_transport_reset_no_sock(), the skb
>+ * does not hold a reference on the socket, and so does not
>+ * transitively hold a reference on the net.
>+ *
>+ * There is an ABA race condition in this sequence:
>+ * 1. the sender sends a packet
>+ * 2. worker calls virtio_transport_recv_pkt(), using the
>+ * sender's net
>+ * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
>+ * sender's net
>+ * 4. virtio_transport_recv_pkt() free's the skb, dropping the
>+ * reference to the socket
>+ * 5. the socket closes, frees its reference to the net
>+ * 6. Finally, the worker for the second t->send_pkt() call
>+ * processes the skb, and uses the now stale net pointer for
>+ * socket lookups.
>+ *
>+ * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
>+ * and hold it until virtio_transport_recv_pkt() completes.
>+ *
>+ * Additionally, we must grab a reference on the skb before
>+ * calling virtio_transport_recv_pkt() to prevent it from
>+ * freeing the skb before we have a chance to release the net.
>+ */
>+ net_mode = virtio_vsock_skb_net_mode(skb);
>+ net = virtio_vsock_skb_net(skb);
Wait, we are adding those just for loopback (in theory used only for
testing/debugging)? And only to support virtio_transport_reset_no_sock()
use case?
Honestly I don't like this, do we have any alternative?
I'll also try to think something else.
Stefano
>+
>+ skb_get(skb);
>+
>+ virtio_transport_recv_pkt(&loopback_transport, skb, net,
>+ net_mode);
>+
>+ virtio_vsock_skb_clear_net(skb);
>+ kfree_skb(skb);
> }
> }
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 08/14] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
From: Stefano Garzarella @ 2025-11-12 14:21 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-8-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:50PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Reject setting VSOCK_NET_MODE_LOCAL with -EOPNOTSUPP if a G2H transport
>is operational. Additionally, reject G2H transport registration if there
>already exists a namespace in local mode.
>
>G2H sockets break in local mode because the G2H transports don't support
>namespacing yet. The current approach is to coerce packets coming out of
>G2H transports into VSOCK_NET_MODE_GLOBAL mode, but it is not possible
>to coerce sockets in the same way because it cannot be deduced which
>transport will be used by the socket. Specifically, when bound to
>VMADDR_CID_ANY in a nested VM (both G2H and H2G available), it is not
>until a packet is received and matched to the bound socket that we
>assign the transport. This presents a chicken-and-egg problem, because
>we need the namespace to lookup the socket and resolve the transport,
>but we need the transport to know how to use the namespace during
>lookup.
>
>For that reason, this patch prevents VSOCK_NET_MODE_LOCAL from being
>used on systems that support G2H, even nested systems that also have H2G
>transports.
>
>Local mode is blocked based on detecting the presence of G2H devices
>(when possible, as hyperv is special). This means that a host kernel
>with G2H support compiled in (or has the module loaded), will still
>support local mode because there is no G2H (e.g., virtio-vsock) device
>detected. This enables using the same kernel in the host and in the
>guest, as we do in kselftest.
>
>Systems with only namespace-aware transports (vhost-vsock, loopback) can
>still use both VSOCK_NET_MODE_GLOBAL and VSOCK_NET_MODE_LOCAL modes as
>intended.
>
>The hyperv transport must be treated specially. Other G2H transports can
>can report presence of a device using get_local_cid(). When a device is
>present it returns a valid CID; otherwise, it returns VMADDR_CID_ANY.
>THe hyperv transport's get_local_cid() always returns VMADDR_CID_ANY,
>however, even when a device is present.
>
>For that reason, this patch adds an always_block_local_mode flag to
>struct vsock_transport. When set to true, VSOCK_NET_MODE_LOCAL is
>blocked unconditionally whenever the transport is registered, regardless
>of device presence. When false, LOCAL mode is only blocked when
>get_local_cid() indicates a device is present (!= VMADDR_CID_ANY).
>
>The hyperv transport sets this flag to true to unconditionally block
>local mode. Other G2H transports (virtio-vsock, vmci-vsock) leave it
>false and continue using device detection via get_local_cid() to block
>local mode.
>
>These restrictions can be lifted in a future patch series when G2H
>transports gain namespace support.
IMO this commit should be before supporting namespaces in any device,
so we are sure we don't have commits where this can happen.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
> include/net/af_vsock.h | 8 +++++++
> net/vmw_vsock/af_vsock.c | 45 +++++++++++++++++++++++++++++++++++++---
> net/vmw_vsock/hyperv_transport.c | 1 +
> 3 files changed, 51 insertions(+), 3 deletions(-)
>
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index cfd121bb5ab7..089c61105dda 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -108,6 +108,14 @@ struct vsock_transport_send_notify_data {
>
> struct vsock_transport {
> struct module *module;
>+ /* If true, block VSOCK_NET_MODE_LOCAL unconditionally when this G2H
>+ * transport is registered. If false, only block LOCAL mode when
>+ * get_local_cid() indicates a device is present (!= VMADDR_CID_ANY).
>+ * Hyperv sets this true because it doesn't offer a callback that
>+ * detects device presence. This only applies to G2H transports; H2G
>+ * transports are unaffected.
>+ */
>+ bool always_block_local_mode;
>
> /* Initialize/tear-down socket. */
> int (*init)(struct vsock_sock *, struct vsock_sock *);
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index c0b5946bdc95..a2da1810b802 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -91,6 +91,11 @@
> * and locked down by a namespace manager. The default is "global". The mode
> * is set per-namespace.
> *
>+ * Note: LOCAL mode is only supported when using namespace-aware transports
>+ * (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
>+ * hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
>+ * with EOPNOTSUPP, as these transports do not support per-namespace
>isolation.
>+ *
> * The modes affect the allocation and accessibility of CIDs as follows:
> *
> * - global - access and allocation are all system-wide
>@@ -2757,12 +2762,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> if (*lenp >= sizeof(data))
> return -EINVAL;
>
>- if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
>+ if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
> mode = VSOCK_NET_MODE_GLOBAL;
>- else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
>+ } else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
>+ /* LOCAL mode is not supported when G2H transports
>+ * (virtio-vsock, hyperv, vmci) are active, because
>+ * these transports don't support namespaces. We must
>+ * stay in GLOBAL mode to avoid bind/lookup mismatches.
>+ *
>+ * Check if G2H transport is present and either:
>+ * 1. Has always_block_local_mode set (hyperv), OR
>+ * 2. Has an actual device present (get_local_cid() != VMADDR_CID_ANY)
>+ */
>+ mutex_lock(&vsock_register_mutex);
>+ if (transport_g2h &&
>+ (transport_g2h->always_block_local_mode ||
>+ transport_g2h->get_local_cid() != VMADDR_CID_ANY)) {
This seems almost like a hack. What about adding a new function in the
transports that tells us whether a device is present or not and
implement it in all of them?
Or a more specific function to check if the transport can work with
local mode (e.g. netns_local_aware() or something like that - I'm not
great with nameming xD)
>+ mutex_unlock(&vsock_register_mutex);
>+ return -EOPNOTSUPP;
>+ }
>+ mutex_unlock(&vsock_register_mutex);
What happen if the G2H is loaded here, just after we release the mutex?
I suspect there could be a race that we may fix postponing the unlock
after the vsock_net_write_mode() call.
WDYT?
> mode = VSOCK_NET_MODE_LOCAL;
>- else
>+ } else {
> return -EINVAL;
>+ }
>
> if (!vsock_net_write_mode(net, mode))
> return -EPERM;
>@@ -2909,6 +2932,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> {
> const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> int err = mutex_lock_interruptible(&vsock_register_mutex);
>+ struct net *net;
>
> if (err)
> return err;
>@@ -2931,6 +2955,21 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> err = -EBUSY;
> goto err_busy;
> }
>+
>+ /* G2H sockets break in LOCAL mode namespaces because G2H transports
>+ * don't support them yet. Block registering new G2H transports if we
>+ * already have local mode namespaces on the system.
>+ */
>+ rcu_read_lock();
>+ for_each_net_rcu(net) {
>+ if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
>+ rcu_read_unlock();
>+ err = -EOPNOTSUPP;
>+ goto err_busy;
>+ }
>+ }
>+ rcu_read_unlock();
>+
> t_g2h = t;
> }
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 432fcbbd14d4..ed48dd1ff19b 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -835,6 +835,7 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
>
> static struct vsock_transport hvs_transport = {
> .module = THIS_MODULE,
>+ .always_block_local_mode = true,
>
> .get_local_cid = hvs_get_local_cid,
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 10/14] selftests/vsock: prepare vm management helpers for namespaces
From: Stefano Garzarella @ 2025-11-12 14:23 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-10-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:52PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add namespace support to vm management, ssh helpers, and vsock_test
>wrapper functions. This enables running VMs and test helpers in specific
>namespaces, which is required for upcoming namespace isolation tests.
>
>The functions still work correctly within the init ns, though the caller
>must now pass "init_ns" explicitly.
>
>No functional changes for existing tests. All have been updated to pass
>"init_ns" explicitly.
>
>Affected functions (such as vm_start() and vm_ssh()) now wrap their
>commands with 'ip netns exec' when executing commands in non-init
>namespaces.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
> tools/testing/selftests/vsock/vmtest.sh | 100 ++++++++++++++++++++++----------
> 1 file changed, 68 insertions(+), 32 deletions(-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index f78cc574c274..663be2da4e22 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -144,7 +144,18 @@ ns_set_mode() {
> }
>
> vm_ssh() {
>- ssh -q -o UserKnownHostsFile=/dev/null -p ${SSH_HOST_PORT} localhost "$@"
>+ local ns_exec
>+
>+ if [[ "${1}" == init_ns ]]; then
>+ ns_exec=""
>+ else
>+ ns_exec="ip netns exec ${1}"
>+ fi
>+
>+ shift
>+
>+ ${ns_exec} ssh -q -o UserKnownHostsFile=/dev/null -p "${SSH_HOST_PORT}" localhost "$@"
>+
> return $?
> }
>
>@@ -267,10 +278,12 @@ terminate_pidfiles() {
>
> vm_start() {
> local pidfile=$1
>+ local ns=$2
> local logfile=/dev/null
> local verbose_opt=""
> local kernel_opt=""
> local qemu_opts=""
>+ local ns_exec=""
> local qemu
>
> qemu=$(command -v "${QEMU}")
>@@ -291,7 +304,11 @@ vm_start() {
> kernel_opt="${KERNEL_CHECKOUT}"
> fi
>
>- vng \
>+ if [[ "${ns}" != "init_ns" ]]; then
>+ ns_exec="ip netns exec ${ns}"
>+ fi
>+
>+ ${ns_exec} vng \
> --run \
> ${kernel_opt} \
> ${verbose_opt} \
>@@ -306,6 +323,7 @@ vm_start() {
> }
>
> vm_wait_for_ssh() {
>+ local ns=$1
> local i
>
> i=0
>@@ -313,7 +331,8 @@ vm_wait_for_ssh() {
> if [[ ${i} -gt ${WAIT_PERIOD_MAX} ]]; then
> die "Timed out waiting for guest ssh"
> fi
>- if vm_ssh -- true; then
>+
>+ if vm_ssh "${ns}" -- true; then
> break
> fi
> i=$(( i + 1 ))
>@@ -347,30 +366,40 @@ wait_for_listener()
> }
>
> vm_wait_for_listener() {
>- local port=$1
>+ local ns=$1
>+ local port=$2
>
>- vm_ssh <<EOF
>+ vm_ssh "${ns}" <<EOF
> $(declare -f wait_for_listener)
> wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX}
> EOF
> }
>
> host_wait_for_listener() {
>- local port=$1
>+ local ns=$1
>+ local port=$2
>
>- wait_for_listener "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}"
>+ if [[ "${ns}" == "init_ns" ]]; then
>+ wait_for_listener "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}"
>+ else
>+ ip netns exec "${ns}" bash <<-EOF
>+ $(declare -f wait_for_listener)
>+ wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX}
>+ EOF
>+ fi
> }
>
> vm_vsock_test() {
>- local host=$1
>- local cid=$2
>- local port=$3
>+ local ns=$1
>+ local host=$2
>+ local cid=$3
>+ local port=$4
> local rc
>
> # log output and use pipefail to respect vsock_test errors
> set -o pipefail
> if [[ "${host}" != server ]]; then
>- vm_ssh -- "${VSOCK_TEST}" \
>+ vm_ssh "${ns}" -- "${VSOCK_TEST}" \
> --mode=client \
> --control-host="${host}" \
> --peer-cid="${cid}" \
>@@ -378,7 +407,7 @@ vm_vsock_test() {
> 2>&1 | log_guest
> rc=$?
> else
>- vm_ssh -- "${VSOCK_TEST}" \
>+ vm_ssh "${ns}" -- "${VSOCK_TEST}" \
> --mode=server \
> --peer-cid="${cid}" \
> --control-port="${port}" \
>@@ -390,7 +419,7 @@ vm_vsock_test() {
> return $rc
> fi
>
>- vm_wait_for_listener "${port}"
>+ vm_wait_for_listener "${ns}" "${port}"
> rc=$?
> fi
> set +o pipefail
>@@ -399,22 +428,28 @@ vm_vsock_test() {
> }
>
> host_vsock_test() {
>- local host=$1
>- local cid=$2
>- local port=$3
>+ local ns=$1
>+ local host=$2
>+ local cid=$3
>+ local port=$4
> local rc
>
>+ local cmd="${VSOCK_TEST}"
>+ if [[ "${ns}" != "init_ns" ]]; then
>+ cmd="ip netns exec ${ns} ${cmd}"
>+ fi
>+
> # log output and use pipefail to respect vsock_test errors
> set -o pipefail
> if [[ "${host}" != server ]]; then
>- ${VSOCK_TEST} \
>+ ${cmd} \
> --mode=client \
> --peer-cid="${cid}" \
> --control-host="${host}" \
> --control-port="${port}" 2>&1 | log_host
> rc=$?
> else
>- ${VSOCK_TEST} \
>+ ${cmd} \
> --mode=server \
> --peer-cid="${cid}" \
> --control-port="${port}" 2>&1 | log_host &
>@@ -425,7 +460,7 @@ host_vsock_test() {
> return $rc
> fi
>
>- host_wait_for_listener "${port}"
>+ host_wait_for_listener "${ns}" "${port}"
> rc=$?
> fi
> set +o pipefail
>@@ -469,11 +504,11 @@ log_guest() {
> }
>
> test_vm_server_host_client() {
>- if ! vm_vsock_test "server" 2 "${TEST_GUEST_PORT}"; then
>+ if ! vm_vsock_test "init_ns" "server" 2 "${TEST_GUEST_PORT}"; then
> return "${KSFT_FAIL}"
> fi
>
>- if ! host_vsock_test "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"; then
>+ if ! host_vsock_test "init_ns" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"; then
> return "${KSFT_FAIL}"
> fi
>
>@@ -481,11 +516,11 @@ test_vm_server_host_client() {
> }
>
> test_vm_client_host_server() {
>- if ! host_vsock_test "server" "${VSOCK_CID}" "${TEST_HOST_PORT_LISTENER}"; then
>+ if ! host_vsock_test "init_ns" "server" "${VSOCK_CID}" "${TEST_HOST_PORT_LISTENER}"; then
> return "${KSFT_FAIL}"
> fi
>
>- if ! vm_vsock_test "10.0.2.2" 2 "${TEST_HOST_PORT_LISTENER}"; then
>+ if ! vm_vsock_test "init_ns" "10.0.2.2" 2 "${TEST_HOST_PORT_LISTENER}"; then
> return "${KSFT_FAIL}"
> fi
>
>@@ -495,13 +530,14 @@ test_vm_client_host_server() {
> test_vm_loopback() {
> local port=60000 # non-forwarded local port
>
>- vm_ssh -- modprobe vsock_loopback &> /dev/null || :
>+ vm_ssh "init_ns" -- modprobe vsock_loopback &> /dev/null || :
>
>- if ! vm_vsock_test "server" 1 "${port}"; then
>+ if ! vm_vsock_test "init_ns" "server" 1 "${port}"; then
> return "${KSFT_FAIL}"
> fi
>
>- if ! vm_vsock_test "127.0.0.1" 1 "${port}"; then
>+
>+ if ! vm_vsock_test "init_ns" "127.0.0.1" 1 "${port}"; then
> return "${KSFT_FAIL}"
> fi
>
>@@ -559,8 +595,8 @@ run_shared_vm_test() {
>
> host_oops_cnt_before=$(dmesg | grep -c -i 'Oops')
> host_warn_cnt_before=$(dmesg --level=warn | grep -c -i 'vsock')
>- vm_oops_cnt_before=$(vm_ssh -- dmesg | grep -c -i 'Oops')
>- vm_warn_cnt_before=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
>+ vm_oops_cnt_before=$(vm_ssh "init_ns" -- dmesg | grep -c -i 'Oops')
>+ vm_warn_cnt_before=$(vm_ssh "init_ns" -- dmesg --level=warn | grep -c -i 'vsock')
>
> name=$(echo "${1}" | awk '{ print $1 }')
> eval test_"${name}"
>@@ -578,13 +614,13 @@ run_shared_vm_test() {
> rc=$KSFT_FAIL
> fi
>
>- vm_oops_cnt_after=$(vm_ssh -- dmesg | grep -i 'Oops' | wc -l)
>+ vm_oops_cnt_after=$(vm_ssh "init_ns" -- dmesg | grep -i 'Oops' | wc -l)
> if [[ ${vm_oops_cnt_after} -gt ${vm_oops_cnt_before} ]]; then
> echo "FAIL: kernel oops detected on vm" | log_host
> rc=$KSFT_FAIL
> fi
>
>- vm_warn_cnt_after=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
>+ vm_warn_cnt_after=$(vm_ssh "init_ns" -- dmesg --level=warn | grep -c -i 'vsock')
> if [[ ${vm_warn_cnt_after} -gt ${vm_warn_cnt_before} ]]; then
> echo "FAIL: kernel warning detected on vm" | log_host
> rc=$KSFT_FAIL
>@@ -630,8 +666,8 @@ cnt_total=0
> if shared_vm_tests_requested "${ARGS[@]}"; then
> log_host "Booting up VM"
> pidfile="$(create_pidfile)"
>- vm_start "${pidfile}"
>- vm_wait_for_ssh
>+ vm_start "${pidfile}" "init_ns"
>+ vm_wait_for_ssh "init_ns"
> log_host "VM booted up"
>
> run_shared_vm_tests "${ARGS[@]}"
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 11/14] selftests/vsock: add tests for proc sys vsock ns_mode
From: Stefano Garzarella @ 2025-11-12 14:38 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-11-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:53PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add tests for the /proc/sys/net/vsock/ns_mode interface. Namely,
>that it accepts "global" and "local" strings and enforces a write-once
>policy.
>
>Start a convention of commenting the test name over the test
>description. Add test name comments over test descriptions that existed
>before this convention.
>
>Add a check_netns() function that checks if the test requires namespaces
>and if the current kernel supports namespaces. Skip tests that require
>namespaces if the system does not have namespace support.
>
>Add a test to verify that guest VMs with an active G2H transport
>(virtio-vsock) cannot set namespace mode to 'local'. This validates
>the mutual exclusion between G2H transports and LOCAL mode.
>
>This patch is the first to add tests that do *not* re-use the same
>shared VM. For that reason, it adds a run_tests() function to run these
>tests and filter out the shared VM tests.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- add test ns_vm_local_mode_rejected to check that guests cannot use
> local mode
>---
> tools/testing/selftests/vsock/vmtest.sh | 130 +++++++++++++++++++++++++++++++-
> 1 file changed, 128 insertions(+), 2 deletions(-)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 663be2da4e22..ef5f1d954f8b 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -41,14 +41,40 @@ readonly KERNEL_CMDLINE="\
> virtme.ssh virtme_ssh_channel=tcp virtme_ssh_user=$USER \
> "
> readonly LOG=$(mktemp /tmp/vsock_vmtest_XXXX.log)
>-readonly TEST_NAMES=(vm_server_host_client vm_client_host_server vm_loopback)
>+readonly TEST_NAMES=(
>+ vm_server_host_client
>+ vm_client_host_server
>+ vm_loopback
>+ ns_host_vsock_ns_mode_ok
>+ ns_host_vsock_ns_mode_write_once_ok
>+ ns_vm_local_mode_rejected
>+)
> readonly TEST_DESCS=(
>+ # vm_server_host_client
> "Run vsock_test in server mode on the VM and in client mode on the host."
>+
>+ # vm_client_host_server
> "Run vsock_test in client mode on the VM and in server mode on the host."
>+
>+ # vm_loopback
> "Run vsock_test using the loopback transport in the VM."
>+
>+ # ns_host_vsock_ns_mode_ok
>+ "Check /proc/sys/net/vsock/ns_mode strings on the host."
>+
>+ # ns_host_vsock_ns_mode_write_once_ok
>+ "Check /proc/sys/net/vsock/ns_mode is write-once on the host."
>+
>+ # ns_vm_local_mode_rejected
>+ "Test that guest VM with G2H transport cannot set namespace mode to 'local'"
> )
>
>-readonly USE_SHARED_VM=(vm_server_host_client vm_client_host_server vm_loopback)
>+readonly USE_SHARED_VM=(
>+ vm_server_host_client
>+ vm_client_host_server
>+ vm_loopback
>+ ns_vm_local_mode_rejected
>+)
> readonly NS_MODES=("local" "global")
>
> VERBOSE=0
>@@ -205,6 +231,20 @@ check_deps() {
> fi
> }
>
>+check_netns() {
>+ local tname=$1
>+
>+ # If the test requires NS support, check if NS support exists
>+ # using /proc/self/ns
>+ if [[ "${tname}" =~ ^ns_ ]] &&
>+ [[ ! -e /proc/self/ns ]]; then
>+ log_host "No NS support detected for test ${tname}"
>+ return 1
>+ fi
>+
>+ return 0
>+}
>+
> check_vng() {
> local tested_versions
> local version
>@@ -503,6 +543,43 @@ log_guest() {
> LOG_PREFIX=guest log "$@"
> }
>
>+test_ns_host_vsock_ns_mode_ok() {
>+ add_namespaces
>+
>+ for mode in "${NS_MODES[@]}"; do
>+ if ! ns_set_mode "${mode}0" "${mode}"; then
>+ del_namespaces
>+ return "${KSFT_FAIL}"
>+ fi
>+ done
>+
>+ del_namespaces
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_host_vsock_ns_mode_write_once_ok() {
>+ add_namespaces
>+
>+ for mode in "${NS_MODES[@]}"; do
>+ local ns="${mode}0"
>+ if ! ns_set_mode "${ns}" "${mode}"; then
>+ del_namespaces
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ # try writing again and expect failure
>+ if ns_set_mode "${ns}" "${mode}"; then
>+ del_namespaces
>+ return "${KSFT_FAIL}"
>+ fi
>+ done
>+
>+ del_namespaces
>+
>+ return "${KSFT_PASS}"
>+}
>+
> test_vm_server_host_client() {
> if ! vm_vsock_test "init_ns" "server" 2 "${TEST_GUEST_PORT}"; then
> return "${KSFT_FAIL}"
>@@ -544,6 +621,26 @@ test_vm_loopback() {
> return "${KSFT_PASS}"
> }
>
>+test_ns_vm_local_mode_rejected() {
>+ # Guest VMs have a G2H transport (virtio-vsock) active, so they
>+ # should not be able to set namespace mode to 'local'.
>+ # This test verifies that the sysctl write fails as expected.
>+
>+ # Try to set local mode in the guest's init_ns
>+ if vm_ssh init_ns "echo local | tee /proc/sys/net/vsock/ns_mode &>/dev/null"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ # Verify mode is still 'global'
>+ local mode
>+ mode=$(vm_ssh init_ns "cat /proc/sys/net/vsock/ns_mode")
>+ if [[ "${mode}" != "global" ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
> shared_vm_test() {
> local tname
>
>@@ -576,6 +673,11 @@ run_shared_vm_tests() {
> continue
> fi
>
>+ if ! check_netns "${arg}"; then
>+ check_result "${KSFT_SKIP}" "${arg}"
>+ continue
>+ fi
>+
> run_shared_vm_test "${arg}"
> check_result "$?" "${arg}"
> done
>@@ -629,6 +731,28 @@ run_shared_vm_test() {
> return "${rc}"
> }
>
>+run_tests() {
>+ for arg in "${ARGS[@]}"; do
>+ if shared_vm_test "${arg}"; then
>+ continue
>+ fi
>+
>+ if ! check_netns "${arg}"; then
>+ check_result "${KSFT_SKIP}" "${arg}"
>+ continue
>+ fi
>+
>+ add_namespaces
Some tests call this in the test function, some not, but we call here
for all test. I'm a bit confused.
Also, are we supposed to use this run_tests() only for namespace tests?
Thanks,
Stefano
>+
>+ name=$(echo "${arg}" | awk '{ print $1 }')
>+ log_host "Executing test_${name}"
>+ eval test_"${name}"
>+ check_result $? "${name}"
>+
>+ del_namespaces
>+ done
>+}
>+
> BUILD=0
> QEMU="qemu-system-$(uname -m)"
>
>@@ -674,6 +798,8 @@ if shared_vm_tests_requested "${ARGS[@]}"; then
> terminate_pidfiles "${pidfile}"
> fi
>
>+run_tests "${ARGS[@]}"
>+
> echo "SUMMARY: PASS=${cnt_pass} SKIP=${cnt_skip} FAIL=${cnt_fail}"
> echo "Log: ${LOG}"
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 13/14] selftests/vsock: add tests for host <-> vm connectivity with namespaces
From: Stefano Garzarella @ 2025-11-12 14:41 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <20251111-vsock-vmtest-v9-13-852787a37bed@meta.com>
On Tue, Nov 11, 2025 at 10:54:55PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add tests to validate namespace correctness using vsock_test and socat.
>The vsock_test tool is used to validate expected success tests, but
>socat is used for expected failure tests. socat is used to ensure that
>connections are rejected outright instead of failing due to some other
>socket behavior (as tested in vsock_test). Additionally, socat is
>already required for tunneling TCP traffic from vsock_test. Using only
>one of the vsock_test tests like 'test_stream_client_close_client' would
>have yielded a similar result, but doing so wouldn't remove the socat
>dependency.
>
>Additionally, check for the dependency socat. socat needs special
>handling beyond just checking if it is on the path because it must be
>compiled with support for both vsock and unix. The function
>check_socat() checks that this support exists.
>
>Add more padding to test name printf strings because the tests added in
>this patch would otherwise overflow.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v9:
>- consistent variable quoting
>---
> tools/testing/selftests/vsock/vmtest.sh | 463 +++++++++++++++++++++++++++++++-
> 1 file changed, 461 insertions(+), 2 deletions(-)
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index cc8dc280afdf..111059924287 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -7,6 +7,7 @@
> # * virtme-ng
> # * busybox-static (used by virtme-ng)
> # * qemu (used by virtme-ng)
>+# * socat
> #
> # shellcheck disable=SC2317,SC2119
>
>@@ -52,6 +53,19 @@ readonly TEST_NAMES=(
> ns_local_same_cid_ok
> ns_global_local_same_cid_ok
> ns_local_global_same_cid_ok
>+ ns_diff_global_host_connect_to_global_vm_ok
>+ ns_diff_global_host_connect_to_local_vm_fails
>+ ns_diff_global_vm_connect_to_global_host_ok
>+ ns_diff_global_vm_connect_to_local_host_fails
>+ ns_diff_local_host_connect_to_local_vm_fails
>+ ns_diff_local_vm_connect_to_local_host_fails
>+ ns_diff_global_to_local_loopback_local_fails
>+ ns_diff_local_to_global_loopback_fails
>+ ns_diff_local_to_local_loopback_fails
>+ ns_diff_global_to_global_loopback_ok
>+ ns_same_local_loopback_ok
>+ ns_same_local_host_connect_to_local_vm_ok
>+ ns_same_local_vm_connect_to_local_host_ok
> )
> readonly TEST_DESCS=(
> # vm_server_host_client
>@@ -82,6 +96,45 @@ readonly TEST_DESCS=(
>
> # ns_local_global_same_cid_ok
> "Check QEMU successfully starts one VM in a local ns and then another VM in a global ns with the same CID."
>+
>+ # ns_diff_global_host_connect_to_global_vm_ok
>+ "Run vsock_test client in global ns with server in VM in another global ns."
>+
>+ # ns_diff_global_host_connect_to_local_vm_fails
>+ "Run socat to test a process in a global ns fails to connect to a VM in a local ns."
>+
>+ # ns_diff_global_vm_connect_to_global_host_ok
>+ "Run vsock_test client in VM in a global ns with server in another global ns."
>+
>+ # ns_diff_global_vm_connect_to_local_host_fails
>+ "Run socat to test a VM in a global ns fails to connect to a host process in a local ns."
>+
>+ # ns_diff_local_host_connect_to_local_vm_fails
>+ "Run socat to test a host process in a local ns fails to connect to a VM in another local ns."
>+
>+ # ns_diff_local_vm_connect_to_local_host_fails
>+ "Run socat to test a VM in a local ns fails to connect to a host process in another local ns."
>+
>+ # ns_diff_global_to_local_loopback_local_fails
>+ "Run socat to test a loopback vsock in a global ns fails to connect to a vsock in a local ns."
>+
>+ # ns_diff_local_to_global_loopback_fails
>+ "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in a global ns."
>+
>+ # ns_diff_local_to_local_loopback_fails
>+ "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in another local ns."
>+
>+ # ns_diff_global_to_global_loopback_ok
>+ "Run socat to test a loopback vsock in a global ns successfully connects to a vsock in another global ns."
>+
>+ # ns_same_local_loopback_ok
>+ "Run socat to test a loopback vsock in a local ns successfully connects to a vsock in the same ns."
>+
>+ # ns_same_local_host_connect_to_local_vm_ok
>+ "Run vsock_test client in a local ns with server in VM in same ns."
>+
>+ # ns_same_local_vm_connect_to_local_host_ok
>+ "Run vsock_test client in VM in a local ns with server in same ns."
> )
>
> readonly USE_SHARED_VM=(
>@@ -113,7 +166,7 @@ usage() {
> for ((i = 0; i < ${#TEST_NAMES[@]}; i++)); do
> name=${TEST_NAMES[${i}]}
> desc=${TEST_DESCS[${i}]}
>- printf "\t%-35s%-35s\n" "${name}" "${desc}"
>+ printf "\t%-55s%-35s\n" "${name}" "${desc}"
> done
> echo
>
>@@ -232,7 +285,7 @@ check_args() {
> }
>
> check_deps() {
>- for dep in vng ${QEMU} busybox pkill ssh; do
>+ for dep in vng ${QEMU} busybox pkill ssh socat; do
> if [[ ! -x $(command -v "${dep}") ]]; then
> echo -e "skip: dependency ${dep} not found!\n"
> exit "${KSFT_SKIP}"
>@@ -283,6 +336,20 @@ check_vng() {
> fi
> }
>
>+check_socat() {
>+ local support_string
>+
>+ support_string="$(socat -V)"
>+
>+ if [[ "${support_string}" != *"WITH_VSOCK 1"* ]]; then
>+ die "err: socat is missing vsock support"
>+ fi
>+
>+ if [[ "${support_string}" != *"WITH_UNIX 1"* ]]; then
>+ die "err: socat is missing unix support"
>+ fi
>+}
>+
> handle_build() {
> if [[ ! "${BUILD}" -eq 1 ]]; then
> return
>@@ -331,6 +398,14 @@ terminate_pidfiles() {
> done
> }
>
>+terminate_pids() {
>+ local pid
>+
>+ for pid in "$@"; do
>+ kill -SIGTERM "${pid}" &>/dev/null || :
>+ done
>+}
>+
> vm_start() {
> local pidfile=$1
> local ns=$2
>@@ -573,6 +648,389 @@ test_ns_host_vsock_ns_mode_ok() {
> return "${KSFT_PASS}"
> }
>
>+test_ns_diff_global_host_connect_to_global_vm_ok() {
>+ local pids pid pidfile
>+ local ns0 ns1 port
>+ declare -a pids
>+ local unixfile
>+ ns0="global0"
>+ ns1="global1"
>+ port=1234
>+ local rc
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ unixfile=$(mktemp -u /tmp/XXXX.sock)
>+ ip netns exec "${ns1}" \
>+ socat TCP-LISTEN:"${TEST_HOST_PORT}",fork \
>+ UNIX-CONNECT:"${unixfile}" &
>+ pids+=($!)
>+ host_wait_for_listener "${ns1}" "${TEST_HOST_PORT}"
>+
>+ ip netns exec "${ns0}" socat UNIX-LISTEN:"${unixfile}",fork \
>+ TCP-CONNECT:localhost:"${TEST_HOST_PORT}" &
>+ pids+=($!)
>+
>+ vm_vsock_test "${ns0}" "server" 2 "${TEST_GUEST_PORT}"
>+ vm_wait_for_listener "${ns0}" "${TEST_GUEST_PORT}"
>+ host_vsock_test "${ns1}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+ rc=$?
>+
>+ for pid in "${pids[@]}"; do
>+ if [[ "$(jobs -p)" = *"${pid}"* ]]; then
>+ kill -SIGTERM "${pid}" &>/dev/null
>+ fi
>+ done
>+
In run_shared_vm_test() we are also checking oops, warn in both host and
VM, should we do the same here in each no-shared test that boot a VM?
I mean, should we generalize run_shared_vm_test() and use it for both
kind of tests?
Stefano
>+ terminate_pidfiles "${pidfile}"
>+
>+ if [[ "${rc}" -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_diff_global_host_connect_to_local_vm_fails() {
>+ local ns0="global0"
>+ local ns1="local0"
>+ local port=12345
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ outfile=$(mktemp)
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns1}"; then
>+ log_host "failed to start vm (cid=${VSOCK_CID}, ns=${ns0})"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns1}"
>+ vm_ssh "${ns1}" -- socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" &
>+ echo TEST | ip netns exec "${ns0}" \
>+ socat STDIN VSOCK-CONNECT:"${VSOCK_CID}":"${port}" 2>/dev/null
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_global_vm_connect_to_global_host_ok() {
>+ local ns0="global0"
>+ local ns1="global1"
>+ local port=12345
>+ local unixfile
>+ local pidfile
>+ local pids
>+
>+ init_namespaces
>+
>+ declare -a pids
>+
>+ log_host "Setup socat bridge from ns ${ns0} to ns ${ns1} over port ${port}"
>+
>+ unixfile=$(mktemp -u /tmp/XXXX.sock)
>+
>+ ip netns exec "${ns0}" \
>+ socat TCP-LISTEN:"${port}" UNIX-CONNECT:"${unixfile}" &
>+ pids+=($!)
>+
>+ ip netns exec "${ns1}" \
>+ socat UNIX-LISTEN:"${unixfile}" TCP-CONNECT:127.0.0.1:"${port}" &
>+ pids+=($!)
>+
>+ log_host "Launching ${VSOCK_TEST} in ns ${ns1}"
>+ host_vsock_test "${ns1}" "server" "${VSOCK_CID}" "${port}"
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ terminate_pids "${pids[@]}"
>+ rm -f "${unixfile}"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+ vm_vsock_test "${ns0}" "10.0.2.2" 2 "${port}"
>+ rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pids[@]}"
>+ rm -f "${unixfile}"
>+
>+ if [[ ! $rc -eq 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+
>+}
>+
>+test_ns_diff_global_vm_connect_to_local_host_fails() {
>+ local ns0="global0"
>+ local ns1="local0"
>+ local port=12345
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ log_host "Launching socat in ns ${ns1}"
>+ outfile=$(mktemp)
>+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT &> "${outfile}" &
>+ pid=$!
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ terminate_pids "${pid}"
>+ rm -f "${outfile}"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+
>+ vm_ssh "${ns0}" -- \
>+ bash -c "echo TEST | socat STDIN VSOCK-CONNECT:2:${port}" 2>&1 | log_guest
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pid}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_host_connect_to_local_vm_fails() {
>+ local ns0="local0"
>+ local ns1="local1"
>+ local port=12345
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ outfile=$(mktemp)
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns1}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns1}"
>+ vm_ssh "${ns1}" -- socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" &
>+ echo TEST | ip netns exec "${ns0}" \
>+ socat STDIN VSOCK-CONNECT:"${VSOCK_CID}":"${port}" 2>/dev/null
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_vm_connect_to_local_host_fails() {
>+ local ns0="local0"
>+ local ns1="local1"
>+ local port=12345
>+ local pidfile
>+ local result
>+ local pid
>+
>+ init_namespaces
>+
>+ log_host "Launching socat in ns ${ns1}"
>+ outfile=$(mktemp)
>+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT &> "${outfile}" &
>+ pid=$!
>+
>+ pidfile="$(create_pidfile)"
>+ if ! vm_start "${pidfile}" "${ns0}"; then
>+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
>+ rm -f "${outfile}"
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_wait_for_ssh "${ns0}"
>+
>+ vm_ssh "${ns0}" -- \
>+ bash -c "echo TEST | socat STDIN VSOCK-CONNECT:2:${port}" 2>&1 | log_guest
>+
>+ terminate_pidfiles "${pidfile}"
>+ terminate_pids "${pid}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" != TEST ]]; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+__test_loopback_two_netns() {
>+ local ns0=$1
>+ local ns1=$2
>+ local port=12345
>+ local result
>+ local pid
>+
>+ modprobe vsock_loopback &> /dev/null || :
>+
>+ log_host "Launching socat in ns ${ns1}"
>+ outfile=$(mktemp)
>+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
>+ pid=$!
>+
>+ log_host "Launching socat in ns ${ns0}"
>+ echo TEST | ip netns exec "${ns0}" socat STDIN VSOCK-CONNECT:1:"${port}" 2>/dev/null
>+ terminate_pids "${pid}"
>+
>+ result=$(cat "${outfile}")
>+ rm -f "${outfile}"
>+
>+ if [[ "${result}" == TEST ]]; then
>+ return 0
>+ fi
>+
>+ return 1
>+}
>+
>+test_ns_diff_global_to_local_loopback_local_fails() {
>+ init_namespaces
>+
>+ if ! __test_loopback_two_netns "global0" "local0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_to_global_loopback_fails() {
>+ init_namespaces
>+
>+ if ! __test_loopback_two_netns "local0" "global0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_local_to_local_loopback_fails() {
>+ init_namespaces
>+
>+ if ! __test_loopback_two_netns "local0" "local1"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_diff_global_to_global_loopback_ok() {
>+ init_namespaces
>+
>+ if __test_loopback_two_netns "global0" "global1"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_same_local_loopback_ok() {
>+ init_namespaces
>+
>+ if __test_loopback_two_netns "local0" "local0"; then
>+ return "${KSFT_PASS}"
>+ fi
>+
>+ return "${KSFT_FAIL}"
>+}
>+
>+test_ns_same_local_host_connect_to_local_vm_ok() {
>+ local ns="local0"
>+ local port=1234
>+ local pidfile
>+ local rc
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+
>+ if ! vm_start "${pidfile}" "${ns}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_vsock_test "${ns}" "server" 2 "${TEST_GUEST_PORT}"
>+ host_vsock_test "${ns}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+ rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ if [[ $rc -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
>+test_ns_same_local_vm_connect_to_local_host_ok() {
>+ local ns="local0"
>+ local port=1234
>+ local pidfile
>+ local rc
>+
>+ init_namespaces
>+
>+ pidfile="$(create_pidfile)"
>+
>+ if ! vm_start "${pidfile}" "${ns}"; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ vm_vsock_test "${ns}" "server" 2 "${TEST_GUEST_PORT}"
>+ host_vsock_test "${ns}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
>+ rc=$?
>+
>+ terminate_pidfiles "${pidfile}"
>+
>+ if [[ $rc -ne 0 ]]; then
>+ return "${KSFT_FAIL}"
>+ fi
>+
>+ return "${KSFT_PASS}"
>+}
>+
> namespaces_can_boot_same_cid() {
> local ns0=$1
> local ns1=$2
>@@ -851,6 +1309,7 @@ fi
> check_args "${ARGS[@]}"
> check_deps
> check_vng
>+check_socat
> handle_build
>
> echo "1..${#ARGS[@]}"
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v9 03/14] vsock/virtio: add netns support to virtio transport and virtio common
From: Bobby Eshleman @ 2025-11-12 16:13 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <cah4sqsqbdp52byutxngl3ko44kduesbhan6luhk3ukzml7bs6@hlv4ckunx7jj>
On Wed, Nov 12, 2025 at 03:18:42PM +0100, Stefano Garzarella wrote:
> On Tue, Nov 11, 2025 at 10:54:45PM -0800, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > Enable network namespace support in the virtio-vsock and common
> > transport layer.
> >
> > The changes include:
>
> This list seems to have been generated by AI. I have nothing against it, but
> I don't think it's important to list all the things that have changed, but
> rather to explain why.
Sounds good, I'll keep that in mind on why vs what. I have been
experimenting with AI in my process, but sadly this list was mostly
hand-rolled. I guess exhaustive listing is an over-correction for too
sparse of commit messages on my part.
>
> > 1. Add a 'net' field to virtio_vsock_pkt_info to carry the namespace
> > pointer for outgoing packets.
>
> Why?
>
> > 2. Add 'net' and 'net_mode' to t->send_pkt() and
> > virtio_transport_recv_pkt() functions
>
> Why?
>
> > 3. Modify callback functions to accept placeholder values
> > (NULL and 0) for net and net_mode. The placeholders will be
>
> Why 0 ? I mean VSOCK_NET_MODE_GLOBAL is also 0, no?
> So I don't understand if you want to specify an invalid value (like NULL) or
> VSOCK_NET_MODE_GLOBAL.
>
> > replaced when later patches in this series add namespace support
> > to transports.
> > 4. Set virtio-vsock to global mode unconditionally, instead of using
> > placeholders. This is done in this patch because virtio-vsock won't
> > have any additional changes to choose the net/net_mode, unlike the
> > other transports. Same complexity as placeholders.
> > 5. Pass net and net_mode to virtio_transport_reset_no_sock() directly.
> > This ensures that the outgoing RST packets are scoped based on the
> > namespace of the receiver of the failed request.
>
> "Receiver" is confusing IMO, see the comment on
> virtio_transport_reset_no_sock().
>
> > 6. Pass net and net_mode to socket lookup functions using
> > vsock_find_{bound,connected}_socket_net().
>
> mmmm, are those functions working fine with the placeholders?
They should resolve everything to global mode as this is why
virtio-vsock does by the end of this series, but I didn't run the
tests specifically on this patch.
>
> If it simplifies, I think we can eventually merge all changes to transports
> that depends on virtio_transport_common in a single commit.
> IMO is better to have working commits than better split.
That would be so much easier. Much of this patch is just me trying to
find a way to keep total patch size reasonably small for review... if
having them all in one commit is preferred then that makes life easier.
The answer to all of the above is that I was just trying to make the
virtio_common changes in one place, but not break bisect/build by
failing to update the transport-level call sites. So the placeholder
values are primarily there to compile.
>
> I mean, is this commit working (at runtime) well?
In theory it should, but I only build checked it.
> >
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> > ---
> > Changes in v9:
> > - include/virtio_vsock.h: send_pkt() cb takes net and net_mode
> > - virtio_transport reset_no_sock() takes net and net_mode
> > - vhost-vsock: add placeholders to recv_pkt() for compilation
> > - loopback: add placeholders to recv_pkt() for compilation
> > - remove skb->cb net/net_mode usage, pass as arguments to
> > t->send_pkt() and virtio_transport_recv_pkt() functions instead.
> > Note that skb->cb will still be used by loopback, but only internal
> > to loopback and never passing it to virtio common.
> > - remove virtio_vsock_alloc_rx_skb(), it is not needed after removing
> > skb->cb usage.
> > - pass net and net_mode to virtio_transport_reset_no_sock()
> >
> > Changes in v8:
> > - add the virtio_vsock_alloc_rx_skb(), to be in same patch that fields
> > are read (Stefano)
> >
> > Changes in v7:
> > - add comment explaining the !vsk case in virtio_transport_alloc_skb()
> > ---
> > drivers/vhost/vsock.c | 6 ++--
> > include/linux/virtio_vsock.h | 8 +++--
> > net/vmw_vsock/virtio_transport.c | 10 ++++--
> > net/vmw_vsock/virtio_transport_common.c | 57 ++++++++++++++++++++++++---------
> > net/vmw_vsock/vsock_loopback.c | 5 +--
> > 5 files changed, 62 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index 34adf0cf9124..0a0e73405532 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -269,7 +269,8 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
> > }
> >
> > static int
> > -vhost_transport_send_pkt(struct sk_buff *skb)
> > +vhost_transport_send_pkt(struct sk_buff *skb, struct net *net,
> > + enum vsock_net_mode net_mode)
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct vhost_vsock *vsock;
> > @@ -537,7 +538,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
> > if (le64_to_cpu(hdr->src_cid) == vsock->guest_cid &&
> > le64_to_cpu(hdr->dst_cid) ==
> > vhost_transport_get_local_cid())
> > - virtio_transport_recv_pkt(&vhost_transport, skb);
> > + virtio_transport_recv_pkt(&vhost_transport, skb, NULL,
> > + 0);
> > else
> > kfree_skb(skb);
> >
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index 0c67543a45c8..5ed6136a4ed4 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -173,6 +173,8 @@ struct virtio_vsock_pkt_info {
> > u32 remote_cid, remote_port;
> > struct vsock_sock *vsk;
> > struct msghdr *msg;
> > + struct net *net;
> > + enum vsock_net_mode net_mode;
> > u32 pkt_len;
> > u16 type;
> > u16 op;
> > @@ -185,7 +187,8 @@ struct virtio_transport {
> > struct vsock_transport transport;
> >
> > /* Takes ownership of the packet */
> > - int (*send_pkt)(struct sk_buff *skb);
> > + int (*send_pkt)(struct sk_buff *skb, struct net *net,
> > + enum vsock_net_mode net_mode);
> >
> > /* Used in MSG_ZEROCOPY mode. Checks, that provided data
> > * (number of buffers) could be transmitted with zerocopy
> > @@ -280,7 +283,8 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
> > void virtio_transport_destruct(struct vsock_sock *vsk);
> >
> > void virtio_transport_recv_pkt(struct virtio_transport *t,
> > - struct sk_buff *skb);
> > + struct sk_buff *skb, struct net *net,
> > + enum vsock_net_mode net_mode);
> > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> > index f92f23be3f59..9395fd875823 100644
> > --- a/net/vmw_vsock/virtio_transport.c
> > +++ b/net/vmw_vsock/virtio_transport.c
> > @@ -231,7 +231,8 @@ static int virtio_transport_send_skb_fast_path(struct virtio_vsock *vsock, struc
> > }
> >
> > static int
> > -virtio_transport_send_pkt(struct sk_buff *skb)
> > +virtio_transport_send_pkt(struct sk_buff *skb, struct net *net,
> > + enum vsock_net_mode net_mode)
> > {
> > struct virtio_vsock_hdr *hdr;
> > struct virtio_vsock *vsock;
> > @@ -660,7 +661,12 @@ static void virtio_transport_rx_work(struct work_struct *work)
> > virtio_vsock_skb_put(skb, payload_len);
> >
> > virtio_transport_deliver_tap_pkt(skb);
> > - virtio_transport_recv_pkt(&virtio_transport, skb);
> > +
> > + /* Force virtio-transport into global mode since it
> > + * does not yet support local-mode namespacing.
> > + */
> > + virtio_transport_recv_pkt(&virtio_transport, skb,
> > + NULL, VSOCK_NET_MODE_GLOBAL);
> > }
> > } while (!virtqueue_enable_cb(vq));
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index dcc8a1d5851e..f4e09cb1567c 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -413,7 +413,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> >
> > virtio_transport_inc_tx_pkt(vvs, skb);
> >
> > - ret = t_ops->send_pkt(skb);
> > + ret = t_ops->send_pkt(skb, info->net, info->net_mode);
> > if (ret < 0)
> > break;
> >
> > @@ -527,6 +527,8 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
> > struct virtio_vsock_pkt_info info = {
> > .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
> > .vsk = vsk,
> > + .net = sock_net(sk_vsock(vsk)),
> > + .net_mode = vsk->net_mode,
> > };
> >
> > return virtio_transport_send_pkt_info(vsk, &info);
> > @@ -1067,6 +1069,8 @@ int virtio_transport_connect(struct vsock_sock *vsk)
> > struct virtio_vsock_pkt_info info = {
> > .op = VIRTIO_VSOCK_OP_REQUEST,
> > .vsk = vsk,
> > + .net = sock_net(sk_vsock(vsk)),
> > + .net_mode = vsk->net_mode,
> > };
> >
> > return virtio_transport_send_pkt_info(vsk, &info);
> > @@ -1082,6 +1086,8 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
> > (mode & SEND_SHUTDOWN ?
> > VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
> > .vsk = vsk,
> > + .net = sock_net(sk_vsock(vsk)),
> > + .net_mode = vsk->net_mode,
> > };
> >
> > return virtio_transport_send_pkt_info(vsk, &info);
> > @@ -1108,6 +1114,8 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
> > .msg = msg,
> > .pkt_len = len,
> > .vsk = vsk,
> > + .net = sock_net(sk_vsock(vsk)),
> > + .net_mode = vsk->net_mode,
> > };
> >
> > return virtio_transport_send_pkt_info(vsk, &info);
> > @@ -1145,6 +1153,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> > .op = VIRTIO_VSOCK_OP_RST,
> > .reply = !!skb,
> > .vsk = vsk,
> > + .net = sock_net(sk_vsock(vsk)),
> > + .net_mode = vsk->net_mode,
> > };
> >
> > /* Send RST only if the original pkt is not a RST pkt */
> > @@ -1156,15 +1166,27 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> >
> > /* Normally packets are associated with a socket. There may be no socket if an
> > * attempt was made to connect to a socket that does not exist.
> > + *
> > + * net and net_mode refer to the net and mode of the receiving device (e.g.,
> > + * vhost_vsock). For loopback, they refer to the sending socket net/mode. This
> > + * way the RST packet is sent back to the same namespace as the bad request.
>
> Could this be a problem, should we split this function?
>
> BTW, I'm a bit confused. For vhost-vsock, this is the namespace of the
> device, so the namespace of the guest, so also in that case the namespace of
> the sender, no?
>
> Maybe sender/receiver are confusing. What you want to highlight with this
> comment?
>
Sounds good, I'll try to update it with clarification. The namespace
passed in needs to be the namespace of whoever sent the bad message.
For vhost-vsock (and probably virtio-vsock eventually) that will be the
device/guest namespace. For loopback, it is just the namespace of the
socket that sent the bad message.
> > */
> > static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> > - struct sk_buff *skb)
> > + struct sk_buff *skb, struct net *net,
> > + enum vsock_net_mode net_mode)
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct virtio_vsock_pkt_info info = {
> > .op = VIRTIO_VSOCK_OP_RST,
> > .type = le16_to_cpu(hdr->type),
> > .reply = true,
> > +
> > + /* net or net_mode are not defined here because we pass
> > + * net and net_mode directly to t->send_pkt(), instead of
> > + * relying on virtio_transport_send_pkt_info() to pass them to
> > + * t->send_pkt(). They are not needed by
> > + * virtio_transport_alloc_skb().
> > + */
> > };
> > struct sk_buff *reply;
> >
> > @@ -1183,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> > if (!reply)
> > return -ENOMEM;
> >
> > - return t->send_pkt(reply);
> > + return t->send_pkt(reply, net, net_mode);
> > }
> >
> > /* This function should be called with sk_lock held and SOCK_DONE set */
> > @@ -1465,6 +1487,8 @@ virtio_transport_send_response(struct vsock_sock *vsk,
> > .remote_port = le32_to_cpu(hdr->src_port),
> > .reply = true,
> > .vsk = vsk,
> > + .net = sock_net(sk_vsock(vsk)),
> > + .net_mode = vsk->net_mode,
> > };
> >
> > return virtio_transport_send_pkt_info(vsk, &info);
> > @@ -1507,12 +1531,12 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> > int ret;
> >
> > if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_REQUEST) {
> > - virtio_transport_reset_no_sock(t, skb);
> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> > return -EINVAL;
> > }
> >
> > if (sk_acceptq_is_full(sk)) {
> > - virtio_transport_reset_no_sock(t, skb);
> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> > return -ENOMEM;
> > }
> >
> > @@ -1520,13 +1544,13 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> > * Subsequent enqueues would lead to a memory leak.
> > */
> > if (sk->sk_shutdown == SHUTDOWN_MASK) {
> > - virtio_transport_reset_no_sock(t, skb);
> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> > return -ESHUTDOWN;
> > }
> >
> > child = vsock_create_connected(sk);
> > if (!child) {
> > - virtio_transport_reset_no_sock(t, skb);
> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> > return -ENOMEM;
> > }
> >
> > @@ -1548,7 +1572,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> > */
> > if (ret || vchild->transport != &t->transport) {
> > release_sock(child);
> > - virtio_transport_reset_no_sock(t, skb);
> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
> > sock_put(child);
> > return ret;
> > }
> > @@ -1576,7 +1600,8 @@ static bool virtio_transport_valid_type(u16 type)
> > * lock.
> > */
> > void virtio_transport_recv_pkt(struct virtio_transport *t,
> > - struct sk_buff *skb)
> > + struct sk_buff *skb, struct net *net,
> > + enum vsock_net_mode net_mode)
> > {
> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> > struct sockaddr_vm src, dst;
> > @@ -1599,24 +1624,24 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> > le32_to_cpu(hdr->fwd_cnt));
> >
> > if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
> > - (void)virtio_transport_reset_no_sock(t, skb);
> > + (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> > goto free_pkt;
> > }
> >
> > /* The socket must be in connected or bound table
> > * otherwise send reset back
> > */
> > - sk = vsock_find_connected_socket(&src, &dst);
> > + sk = vsock_find_connected_socket_net(&src, &dst, net, net_mode);
>
> Here `net` can be null, right? Is this okay?
>
Yes, it can be null. net_eq() comparisons pointers (returns false), and
then the modes evaluate w/ GLOBAL == GLOBAL.
This goes away if we combine patches though.
Thanks again for the review!
Best,
Bobby
^ permalink raw reply
* RE: [PATCH v4] mshv: Extend create partition ioctl to support cpu features
From: Michael Kelley @ 2025-11-12 16:27 UTC (permalink / raw)
To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, wei.liu@kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, decui@microsoft.com,
longli@microsoft.com, skinsburskii@linux.microsoft.com,
prapal@linux.microsoft.com, mrathor@linux.microsoft.com,
muislam@microsoft.com, anrayabh@linux.microsoft.com, Jinank Jain
In-Reply-To: <1762903194-25195-1-git-send-email-nunodasneves@linux.microsoft.com>
From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, November 11, 2025 3:20 PM
>
> The existing mshv create partition ioctl does not provide a way to
> specify which cpu features are enabled in the guest. Instead, it
> attempts to enable all features and those that are not supported are
> silently disabled by the hypervisor.
>
> This was done to reduce unnecessary complexity and is sufficient for
> many cases. However, new scenarios require fine-grained control over
> these features.
>
> Define a new mshv_create_partition_v2 structure which supports
> passing the disabled processor and xsave feature bits through to the
> create partition hypercall directly.
>
> Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
> the new structure. If unset, the original mshv_create_partition struct
> is used, with the old behavior of enabling all features.
>
> Co-developed-by: Jinank Jain <jinankjain@microsoft.com>
> Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
> Signed-off-by: Muminul Islam <muislam@microsoft.com>
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> Changes in v4:
> - Change BIT() to BIT_ULL() [Michael Kelley]
> - Enforce pt_num_cpu_fbanks == MSHV_NUM_CPU_FEATURES_BANKS and expect
> that number to never change. In future, additional processor banks
> will be settable as 'early' partition properties. Remove redundant
> code that set default values for unspecified banks [Michael Kelley]
> - Set xsave features to 0 on arm64 [Michael Kelley]
> - Add clarifying comments in a few places
>
> Changes in v3:
> - Remove the new cpu features definitions in hvhdk.h, and retain the
> old behavior of enabling all features for the old struct. For the v2
> struct, still disable unspecified feature banks, since that makes it
> robust to future extensions.
> - Amend comments and commit message to reflect the above
> - Fix unused variable on arm64 [kernel test robot]
>
> Changes in v2:
> - Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
> - Fix compilation issue on arm64 [kernel test robot]
> ---
> drivers/hv/mshv_root_main.c | 113 +++++++++++++++++++++++++++++-------
> include/uapi/linux/mshv.h | 34 +++++++++++
> 2 files changed, 126 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index d542a0143bb8..9f9438289b60 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1900,43 +1900,114 @@ add_partition(struct mshv_partition *partition)
> return 0;
> }
>
> -static long
> -mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> +static_assert(MSHV_NUM_CPU_FEATURES_BANKS ==
> + HV_PARTITION_PROCESSOR_FEATURES_BANKS);
> +
> +static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
> + struct hv_partition_creation_properties *cr_props,
> + union hv_partition_isolation_properties *isol_props)
> {
> - struct mshv_create_partition args;
> - u64 creation_flags;
> - struct hv_partition_creation_properties creation_properties = {};
> - union hv_partition_isolation_properties isolation_properties = {};
> - struct mshv_partition *partition;
> - struct file *file;
> - int fd;
> - long ret;
> + int i;
> + struct mshv_create_partition_v2 args;
> + union hv_partition_processor_features *disabled_procs;
> + union hv_partition_processor_xsave_features *disabled_xsave;
>
> - if (copy_from_user(&args, user_arg, sizeof(args)))
> + /* First, copy v1 struct in case user is on previous versions */
> + if (copy_from_user(&args, user_arg,
> + sizeof(struct mshv_create_partition)))
> return -EFAULT;
>
> if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
> args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
> return -EINVAL;
>
> + disabled_procs = &cr_props->disabled_processor_features;
> + disabled_xsave = &cr_props->disabled_processor_xsave_features;
> +
> + /* Check if user provided newer struct with feature fields */
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
> + if (copy_from_user(&args, user_arg, sizeof(args)))
> + return -EFAULT;
There's subtle issue here that I didn't notice previously. This second copy_from_user()
re-populates the first two fields of the "args" local variable. These two fields were
validated by code a few lines above. But there's no guarantee that a second read of
user space will get the same values. User space could have another thread that
changes the user space values between the two copy_from_user() calls, and thereby
sneak in some bogus values to be used further down in this function. Because of
this risk, there's a general rule for kernel code, which is to avoid multiple accesses to
the same user space values. There are places in the kernel where such double reads
would be an exploitable security hole.
The fix would be to validate the pt_flags and pt_isolation fields again, or to have the
second copy_from_user copy only the additional fields. But it's also the case that the
way the pt_flags and pt_isolation fields are used further down in this function,
nothing bad can happen if malicious user space should succeed in sneaking in some
bogus values.
Net, as currently coded, there's nothing that needs to be fixed. It would be more
robust to do one of the two fixes, if for no other reason than to acknowledge
awareness of the risk of reading user space twice. But I'm not going to insist
on a respin.
> +
> + if (args.pt_num_cpu_fbanks != MSHV_NUM_CPU_FEATURES_BANKS ||
> + mshv_field_nonzero(args, pt_rsvd) ||
> + mshv_field_nonzero(args, pt_rsvd1))
> + return -EINVAL;
> +
> + /*
> + * Note this assumes MSHV_NUM_CPU_FEATURES_BANKS will never
> + * change and equals HV_PARTITION_PROCESSOR_FEATURES_BANKS
> + * (i.e. 2).
> + *
> + * Further banks (index >= 2) will be modifiable as 'early'
> + * properties via the set partition property hypercall.
> + */
> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> + disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
> +
> +#if IS_ENABLED(CONFIG_X86_64)
> + disabled_xsave->as_uint64 = args.pt_disabled_xsave;
> +#else
> + /*
> + * In practice this field is ignored on arm64, but safer to
> + * zero it in case it is ever used.
> + */
> + disabled_xsave->as_uint64 = 0;
> +
> + if (mshv_field_nonzero(args, pt_rsvd2))
> + return -EINVAL;
> +#endif
> + } else {
> + /*
> + * v1 behavior: try to enable everything. The hypervisor will
> + * disable features that are not supported. The banks can be
> + * queried via the get partition property hypercall.
> + */
> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> + disabled_procs->as_uint64[i] = 0;
> +
> + disabled_xsave->as_uint64 = 0;
> + }
> +
> /* Only support EXO partitions */
> - creation_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
> - HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
> + *pt_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
> + HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
> +
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_LAPIC))
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_X2APIC))
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
> + if (args.pt_flags & BIT_ULL(MSHV_PT_BIT_GPA_SUPER_PAGES))
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>
> - if (args.pt_flags & BIT(MSHV_PT_BIT_LAPIC))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
> - if (args.pt_flags & BIT(MSHV_PT_BIT_X2APIC))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
> - if (args.pt_flags & BIT(MSHV_PT_BIT_GPA_SUPER_PAGES))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
> + isol_props->as_uint64 = 0;
>
> switch (args.pt_isolation) {
> case MSHV_PT_ISOLATION_NONE:
> - isolation_properties.isolation_type =
> - HV_PARTITION_ISOLATION_TYPE_NONE;
> + isol_props->isolation_type = HV_PARTITION_ISOLATION_TYPE_NONE;
> break;
> }
>
> + return 0;
> +}
> +
> +static long
> +mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> +{
> + u64 creation_flags;
> + struct hv_partition_creation_properties creation_properties;
> + union hv_partition_isolation_properties isolation_properties;
> + struct mshv_partition *partition;
> + struct file *file;
> + int fd;
> + long ret;
> +
> + ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
> + &creation_properties,
> + &isolation_properties);
> + if (ret)
> + return ret;
> +
> partition = kzalloc(sizeof(*partition), GFP_KERNEL);
> if (!partition)
> return -ENOMEM;
> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
> index 876bfe4e4227..cf904f3aa201 100644
> --- a/include/uapi/linux/mshv.h
> +++ b/include/uapi/linux/mshv.h
> @@ -26,6 +26,7 @@ enum {
> MSHV_PT_BIT_LAPIC,
> MSHV_PT_BIT_X2APIC,
> MSHV_PT_BIT_GPA_SUPER_PAGES,
> + MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES,
> MSHV_PT_BIT_COUNT,
> };
>
> @@ -41,6 +42,8 @@ enum {
> * @pt_flags: Bitmask of 1 << MSHV_PT_BIT_*
> * @pt_isolation: MSHV_PT_ISOLATION_*
> *
> + * This is the initial/v1 version for backward compatibility.
> + *
> * Returns a file descriptor to act as a handle to a guest partition.
> * At this point the partition is not yet initialized in the hypervisor.
> * Some operations must be done with the partition in this state, e.g. setting
> @@ -52,6 +55,37 @@ struct mshv_create_partition {
> __u64 pt_isolation;
> };
>
> +#define MSHV_NUM_CPU_FEATURES_BANKS 2
> +
> +/**
> + * struct mshv_create_partition_v2
> + *
> + * This is extended version of the above initial MSHV_CREATE_PARTITION
> + * ioctl and allows for following additional parameters:
> + *
> + * @pt_num_cpu_fbanks: Must be set to MSHV_NUM_CPU_FEATURES_BANKS.
> + * @pt_cpu_fbanks: Disabled processor feature banks array.
> + * @pt_disabled_xsave: Disabled xsave feature bits.
> + *
> + * pt_cpu_fbanks and pt_disabled_xsave are passed through as-is to the create
> + * partition hypercall.
> + *
> + * Returns : same as above original mshv_create_partition
> + */
> +struct mshv_create_partition_v2 {
> + __u64 pt_flags;
> + __u64 pt_isolation;
> + __u16 pt_num_cpu_fbanks;
> + __u8 pt_rsvd[6]; /* MBZ */
> + __u64 pt_cpu_fbanks[MSHV_NUM_CPU_FEATURES_BANKS];
> + __u64 pt_rsvd1[2]; /* MBZ */
> +#if defined(__x86_64__)
> + __u64 pt_disabled_xsave;
> +#else
> + __u64 pt_rsvd2; /* MBZ */
> +#endif
> +} __packed;
> +
> /* /dev/mshv */
> #define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct mshv_create_partition)
>
> --
> 2.34.1
Other than the double read of user space, LGTM.
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* Re: [PATCH net-next v9 03/14] vsock/virtio: add netns support to virtio transport and virtio common
From: Stefano Garzarella @ 2025-11-12 17:39 UTC (permalink / raw)
To: Bobby Eshleman
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <aRSyPqNo1LhqGLBq@devvm11784.nha0.facebook.com>
On Wed, Nov 12, 2025 at 08:13:50AM -0800, Bobby Eshleman wrote:
>On Wed, Nov 12, 2025 at 03:18:42PM +0100, Stefano Garzarella wrote:
>> On Tue, Nov 11, 2025 at 10:54:45PM -0800, Bobby Eshleman wrote:
>> > From: Bobby Eshleman <bobbyeshleman@meta.com>
>> >
>> > Enable network namespace support in the virtio-vsock and common
>> > transport layer.
>> >
>> > The changes include:
>>
>> This list seems to have been generated by AI. I have nothing against it, but
>> I don't think it's important to list all the things that have changed, but
>> rather to explain why.
>
>Sounds good, I'll keep that in mind on why vs what. I have been
>experimenting with AI in my process, but sadly this list was mostly
>hand-rolled. I guess exhaustive listing is an over-correction for too
>sparse of commit messages on my part.
>
>>
>> > 1. Add a 'net' field to virtio_vsock_pkt_info to carry the namespace
>> > pointer for outgoing packets.
>>
>> Why?
>>
>> > 2. Add 'net' and 'net_mode' to t->send_pkt() and
>> > virtio_transport_recv_pkt() functions
>>
>> Why?
>>
>> > 3. Modify callback functions to accept placeholder values
>> > (NULL and 0) for net and net_mode. The placeholders will be
>>
>> Why 0 ? I mean VSOCK_NET_MODE_GLOBAL is also 0, no?
>> So I don't understand if you want to specify an invalid value (like NULL) or
>> VSOCK_NET_MODE_GLOBAL.
>>
>> > replaced when later patches in this series add namespace support
>> > to transports.
>> > 4. Set virtio-vsock to global mode unconditionally, instead of using
>> > placeholders. This is done in this patch because virtio-vsock won't
>> > have any additional changes to choose the net/net_mode, unlike the
>> > other transports. Same complexity as placeholders.
>> > 5. Pass net and net_mode to virtio_transport_reset_no_sock() directly.
>> > This ensures that the outgoing RST packets are scoped based on the
>> > namespace of the receiver of the failed request.
>>
>> "Receiver" is confusing IMO, see the comment on
>> virtio_transport_reset_no_sock().
>>
>> > 6. Pass net and net_mode to socket lookup functions using
>> > vsock_find_{bound,connected}_socket_net().
>>
>> mmmm, are those functions working fine with the placeholders?
>
>They should resolve everything to global mode as this is why
>virtio-vsock does by the end of this series, but I didn't run the
>tests specifically on this patch.
>>
>> If it simplifies, I think we can eventually merge all changes to transports
>> that depends on virtio_transport_common in a single commit.
>> IMO is better to have working commits than better split.
>
>That would be so much easier. Much of this patch is just me trying to
>find a way to keep total patch size reasonably small for review... if
>having them all in one commit is preferred then that makes life easier.
>
>The answer to all of the above is that I was just trying to make the
>virtio_common changes in one place, but not break bisect/build by
>failing to update the transport-level call sites. So the placeholder
>values are primarily there to compile.
In theory, they should compile, but they should also properly behave.
BTW I strongly believe that having separate commits is a great thing,
but we shouldn't take things to extremes and complicate our lives when
things are too closely related, as in this case.
There is a clear dependency between these patches, so IMO, if the patch
doesn't become huge, it's better to have everything together. (I mean
between dependencies with virtio_transport_common).
What we could perhaps do is have an initial commit where you make the
changes, but the behavior remains unchanged (continue to use global
everywhere, as for virtio_transport.c in this patch), and then specific
commits to just enable support for local/global.
Not sure if it's doable, but I'd like to remove the placeholders if
possibile. Let's discuss more about it if there are issues.
>
>>
>> I mean, is this commit working (at runtime) well?
>
>In theory it should, but I only build checked it.
>
>> >
>> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>> > ---
>> > Changes in v9:
>> > - include/virtio_vsock.h: send_pkt() cb takes net and net_mode
>> > - virtio_transport reset_no_sock() takes net and net_mode
>> > - vhost-vsock: add placeholders to recv_pkt() for compilation
>> > - loopback: add placeholders to recv_pkt() for compilation
>> > - remove skb->cb net/net_mode usage, pass as arguments to
>> > t->send_pkt() and virtio_transport_recv_pkt() functions instead.
>> > Note that skb->cb will still be used by loopback, but only internal
>> > to loopback and never passing it to virtio common.
>> > - remove virtio_vsock_alloc_rx_skb(), it is not needed after removing
>> > skb->cb usage.
>> > - pass net and net_mode to virtio_transport_reset_no_sock()
>> >
>> > Changes in v8:
>> > - add the virtio_vsock_alloc_rx_skb(), to be in same patch that fields
>> > are read (Stefano)
>> >
>> > Changes in v7:
>> > - add comment explaining the !vsk case in virtio_transport_alloc_skb()
>> > ---
>> > drivers/vhost/vsock.c | 6 ++--
>> > include/linux/virtio_vsock.h | 8 +++--
>> > net/vmw_vsock/virtio_transport.c | 10 ++++--
>> > net/vmw_vsock/virtio_transport_common.c | 57 ++++++++++++++++++++++++---------
>> > net/vmw_vsock/vsock_loopback.c | 5 +--
>> > 5 files changed, 62 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>> > index 34adf0cf9124..0a0e73405532 100644
>> > --- a/drivers/vhost/vsock.c
>> > +++ b/drivers/vhost/vsock.c
>> > @@ -269,7 +269,8 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
>> > }
>> >
>> > static int
>> > -vhost_transport_send_pkt(struct sk_buff *skb)
>> > +vhost_transport_send_pkt(struct sk_buff *skb, struct net *net,
>> > + enum vsock_net_mode net_mode)
>> > {
>> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
>> > struct vhost_vsock *vsock;
>> > @@ -537,7 +538,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
>> > if (le64_to_cpu(hdr->src_cid) == vsock->guest_cid &&
>> > le64_to_cpu(hdr->dst_cid) ==
>> > vhost_transport_get_local_cid())
>> > - virtio_transport_recv_pkt(&vhost_transport, skb);
>> > + virtio_transport_recv_pkt(&vhost_transport, skb, NULL,
>> > + 0);
>> > else
>> > kfree_skb(skb);
>> >
>> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> > index 0c67543a45c8..5ed6136a4ed4 100644
>> > --- a/include/linux/virtio_vsock.h
>> > +++ b/include/linux/virtio_vsock.h
>> > @@ -173,6 +173,8 @@ struct virtio_vsock_pkt_info {
>> > u32 remote_cid, remote_port;
>> > struct vsock_sock *vsk;
>> > struct msghdr *msg;
>> > + struct net *net;
>> > + enum vsock_net_mode net_mode;
>> > u32 pkt_len;
>> > u16 type;
>> > u16 op;
>> > @@ -185,7 +187,8 @@ struct virtio_transport {
>> > struct vsock_transport transport;
>> >
>> > /* Takes ownership of the packet */
>> > - int (*send_pkt)(struct sk_buff *skb);
>> > + int (*send_pkt)(struct sk_buff *skb, struct net *net,
>> > + enum vsock_net_mode net_mode);
>> >
>> > /* Used in MSG_ZEROCOPY mode. Checks, that provided data
>> > * (number of buffers) could be transmitted with zerocopy
>> > @@ -280,7 +283,8 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
>> > void virtio_transport_destruct(struct vsock_sock *vsk);
>> >
>> > void virtio_transport_recv_pkt(struct virtio_transport *t,
>> > - struct sk_buff *skb);
>> > + struct sk_buff *skb, struct net *net,
>> > + enum vsock_net_mode net_mode);
>> > void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
>> > u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
>> > void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>> > diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>> > index f92f23be3f59..9395fd875823 100644
>> > --- a/net/vmw_vsock/virtio_transport.c
>> > +++ b/net/vmw_vsock/virtio_transport.c
>> > @@ -231,7 +231,8 @@ static int virtio_transport_send_skb_fast_path(struct virtio_vsock *vsock, struc
>> > }
>> >
>> > static int
>> > -virtio_transport_send_pkt(struct sk_buff *skb)
>> > +virtio_transport_send_pkt(struct sk_buff *skb, struct net *net,
>> > + enum vsock_net_mode net_mode)
>> > {
>> > struct virtio_vsock_hdr *hdr;
>> > struct virtio_vsock *vsock;
>> > @@ -660,7 +661,12 @@ static void virtio_transport_rx_work(struct work_struct *work)
>> > virtio_vsock_skb_put(skb, payload_len);
>> >
>> > virtio_transport_deliver_tap_pkt(skb);
>> > - virtio_transport_recv_pkt(&virtio_transport, skb);
>> > +
>> > + /* Force virtio-transport into global mode since it
>> > + * does not yet support local-mode namespacing.
>> > + */
>> > + virtio_transport_recv_pkt(&virtio_transport, skb,
>> > + NULL, VSOCK_NET_MODE_GLOBAL);
>> > }
>> > } while (!virtqueue_enable_cb(vq));
>> >
>> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> > index dcc8a1d5851e..f4e09cb1567c 100644
>> > --- a/net/vmw_vsock/virtio_transport_common.c
>> > +++ b/net/vmw_vsock/virtio_transport_common.c
>> > @@ -413,7 +413,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>> >
>> > virtio_transport_inc_tx_pkt(vvs, skb);
>> >
>> > - ret = t_ops->send_pkt(skb);
>> > + ret = t_ops->send_pkt(skb, info->net, info->net_mode);
>> > if (ret < 0)
>> > break;
>> >
>> > @@ -527,6 +527,8 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
>> > struct virtio_vsock_pkt_info info = {
>> > .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
>> > .vsk = vsk,
>> > + .net = sock_net(sk_vsock(vsk)),
>> > + .net_mode = vsk->net_mode,
>> > };
>> >
>> > return virtio_transport_send_pkt_info(vsk, &info);
>> > @@ -1067,6 +1069,8 @@ int virtio_transport_connect(struct vsock_sock *vsk)
>> > struct virtio_vsock_pkt_info info = {
>> > .op = VIRTIO_VSOCK_OP_REQUEST,
>> > .vsk = vsk,
>> > + .net = sock_net(sk_vsock(vsk)),
>> > + .net_mode = vsk->net_mode,
>> > };
>> >
>> > return virtio_transport_send_pkt_info(vsk, &info);
>> > @@ -1082,6 +1086,8 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
>> > (mode & SEND_SHUTDOWN ?
>> > VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
>> > .vsk = vsk,
>> > + .net = sock_net(sk_vsock(vsk)),
>> > + .net_mode = vsk->net_mode,
>> > };
>> >
>> > return virtio_transport_send_pkt_info(vsk, &info);
>> > @@ -1108,6 +1114,8 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
>> > .msg = msg,
>> > .pkt_len = len,
>> > .vsk = vsk,
>> > + .net = sock_net(sk_vsock(vsk)),
>> > + .net_mode = vsk->net_mode,
>> > };
>> >
>> > return virtio_transport_send_pkt_info(vsk, &info);
>> > @@ -1145,6 +1153,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
>> > .op = VIRTIO_VSOCK_OP_RST,
>> > .reply = !!skb,
>> > .vsk = vsk,
>> > + .net = sock_net(sk_vsock(vsk)),
>> > + .net_mode = vsk->net_mode,
>> > };
>> >
>> > /* Send RST only if the original pkt is not a RST pkt */
>> > @@ -1156,15 +1166,27 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
>> >
>> > /* Normally packets are associated with a socket. There may be no socket if an
>> > * attempt was made to connect to a socket that does not exist.
>> > + *
>> > + * net and net_mode refer to the net and mode of the receiving device (e.g.,
>> > + * vhost_vsock). For loopback, they refer to the sending socket net/mode. This
>> > + * way the RST packet is sent back to the same namespace as the bad request.
>>
>> Could this be a problem, should we split this function?
>>
>> BTW, I'm a bit confused. For vhost-vsock, this is the namespace of the
>> device, so the namespace of the guest, so also in that case the namespace of
>> the sender, no?
>>
>> Maybe sender/receiver are confusing. What you want to highlight with this
>> comment?
>>
>
>Sounds good, I'll try to update it with clarification. The namespace
>passed in needs to be the namespace of whoever sent the bad message.
>For vhost-vsock (and probably virtio-vsock eventually) that will be the
>device/guest namespace. For loopback, it is just the namespace of the
>socket that sent the bad message.
Yeah now is clear, thanks! So, IMO the `receiving device` was a bit
confusing.
>
>> > */
>> > static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>> > - struct sk_buff *skb)
>> > + struct sk_buff *skb, struct net *net,
>> > + enum vsock_net_mode net_mode)
>> > {
>> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
>> > struct virtio_vsock_pkt_info info = {
>> > .op = VIRTIO_VSOCK_OP_RST,
>> > .type = le16_to_cpu(hdr->type),
>> > .reply = true,
>> > +
>> > + /* net or net_mode are not defined here because we pass
>> > + * net and net_mode directly to t->send_pkt(), instead of
>> > + * relying on virtio_transport_send_pkt_info() to pass them to
>> > + * t->send_pkt(). They are not needed by
>> > + * virtio_transport_alloc_skb().
>> > + */
>> > };
>> > struct sk_buff *reply;
>> >
>> > @@ -1183,7 +1205,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>> > if (!reply)
>> > return -ENOMEM;
>> >
>> > - return t->send_pkt(reply);
>> > + return t->send_pkt(reply, net, net_mode);
>> > }
>> >
>> > /* This function should be called with sk_lock held and SOCK_DONE set */
>> > @@ -1465,6 +1487,8 @@ virtio_transport_send_response(struct vsock_sock *vsk,
>> > .remote_port = le32_to_cpu(hdr->src_port),
>> > .reply = true,
>> > .vsk = vsk,
>> > + .net = sock_net(sk_vsock(vsk)),
>> > + .net_mode = vsk->net_mode,
>> > };
>> >
>> > return virtio_transport_send_pkt_info(vsk, &info);
>> > @@ -1507,12 +1531,12 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> > int ret;
>> >
>> > if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_REQUEST) {
>> > - virtio_transport_reset_no_sock(t, skb);
>> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
>> > return -EINVAL;
>> > }
>> >
>> > if (sk_acceptq_is_full(sk)) {
>> > - virtio_transport_reset_no_sock(t, skb);
>> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
>> > return -ENOMEM;
>> > }
>> >
>> > @@ -1520,13 +1544,13 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> > * Subsequent enqueues would lead to a memory leak.
>> > */
>> > if (sk->sk_shutdown == SHUTDOWN_MASK) {
>> > - virtio_transport_reset_no_sock(t, skb);
>> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
>> > return -ESHUTDOWN;
>> > }
>> >
>> > child = vsock_create_connected(sk);
>> > if (!child) {
>> > - virtio_transport_reset_no_sock(t, skb);
>> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
>> > return -ENOMEM;
>> > }
>> >
>> > @@ -1548,7 +1572,7 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>> > */
>> > if (ret || vchild->transport != &t->transport) {
>> > release_sock(child);
>> > - virtio_transport_reset_no_sock(t, skb);
>> > + virtio_transport_reset_no_sock(t, skb, sock_net(sk), vsk->net_mode);
>> > sock_put(child);
>> > return ret;
>> > }
>> > @@ -1576,7 +1600,8 @@ static bool virtio_transport_valid_type(u16 type)
>> > * lock.
>> > */
>> > void virtio_transport_recv_pkt(struct virtio_transport *t,
>> > - struct sk_buff *skb)
>> > + struct sk_buff *skb, struct net *net,
>> > + enum vsock_net_mode net_mode)
>> > {
>> > struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
>> > struct sockaddr_vm src, dst;
>> > @@ -1599,24 +1624,24 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
>> > le32_to_cpu(hdr->fwd_cnt));
>> >
>> > if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
>> > - (void)virtio_transport_reset_no_sock(t, skb);
>> > + (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
>> > goto free_pkt;
>> > }
>> >
>> > /* The socket must be in connected or bound table
>> > * otherwise send reset back
>> > */
>> > - sk = vsock_find_connected_socket(&src, &dst);
>> > + sk = vsock_find_connected_socket_net(&src, &dst, net, net_mode);
>>
>> Here `net` can be null, right? Is this okay?
>>
>
>Yes, it can be null. net_eq() comparisons pointers (returns false), and
>then the modes evaluate w/ GLOBAL == GLOBAL.
>
>This goes away if we combine patches though.
I see, thanks!
Stefano
^ permalink raw reply
* Re: [PATCH net-next v9 06/14] vsock/loopback: add netns support
From: Bobby Eshleman @ 2025-11-12 18:27 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <g6bxp6hketbjrddzni2ln37gsezqvxbu2orheorzh7fs66roll@hhcrgsos3ui3>
On Wed, Nov 12, 2025 at 03:19:47PM +0100, Stefano Garzarella wrote:
> On Tue, Nov 11, 2025 at 10:54:48PM -0800, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > Add NS support to vsock loopback. Sockets in a global mode netns
> > communicate with each other, regardless of namespace. Sockets in a local
> > mode netns may only communicate with other sockets within the same
> > namespace.
> >
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> > ---
> > Changes in v9:
> > - remove per-netns vsock_loopback and workqueues, just re-using
> > the net and net_mode in skb->cb achieved the same result in a simpler
> > way. Also removed need for pernet_subsys.
> > - properly track net references
> >
> > Changes in v7:
> > - drop for_each_net() init/exit, drop net_rwsem, the pernet registration
> > handles this automatically and race-free
> > - flush workqueue before destruction, purge pkt list
> > - remember net_mode instead of current net mode
> > - keep space after INIT_WORK()
> > - change vsock_loopback in netns_vsock to ->priv void ptr
> > - rename `orig_net_mode` to `net_mode`
> > - remove useless comment
> > - protect `register_pernet_subsys()` with `net_rwsem`
> > - do cleanup before releasing `net_rwsem` when failure happens
> > - call `unregister_pernet_subsys()` in `vsock_loopback_exit()`
> > - call `vsock_loopback_deinit_vsock()` in `vsock_loopback_exit()`
> >
> > Changes in v6:
> > - init pernet ops for vsock_loopback module
> > - vsock_loopback: add space in struct to clarify lock protection
> > - do proper cleanup/unregister on vsock_loopback_exit()
> > - vsock_loopback: use virtio_vsock_skb_net()
> >
> > Changes in v5:
> > - add callbacks code to avoid reverse dependency
> > - add logic for handling vsock_loopback setup for already existing
> > namespaces
> > ---
> > net/vmw_vsock/vsock_loopback.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> > index d3ac056663ea..e62f6c516992 100644
> > --- a/net/vmw_vsock/vsock_loopback.c
> > +++ b/net/vmw_vsock/vsock_loopback.c
> > @@ -32,6 +32,9 @@ static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
> > struct vsock_loopback *vsock = &the_vsock_loopback;
> > int len = skb->len;
> >
> > + virtio_vsock_skb_set_net(skb, net);
> > + virtio_vsock_skb_set_net_mode(skb, net_mode);
> > +
> > virtio_vsock_skb_queue_tail(&vsock->pkt_queue, skb);
> > queue_work(vsock->workqueue, &vsock->pkt_work);
> >
> > @@ -116,8 +119,10 @@ static void vsock_loopback_work(struct work_struct *work)
> > {
> > struct vsock_loopback *vsock =
> > container_of(work, struct vsock_loopback, pkt_work);
> > + enum vsock_net_mode net_mode;
> > struct sk_buff_head pkts;
> > struct sk_buff *skb;
> > + struct net *net;
> >
> > skb_queue_head_init(&pkts);
> >
> > @@ -131,7 +136,41 @@ static void vsock_loopback_work(struct work_struct *work)
> > */
> > virtio_transport_consume_skb_sent(skb, false);
> > virtio_transport_deliver_tap_pkt(skb);
> > - virtio_transport_recv_pkt(&loopback_transport, skb, NULL, 0);
> > +
> > + /* In the case of virtio_transport_reset_no_sock(), the skb
> > + * does not hold a reference on the socket, and so does not
> > + * transitively hold a reference on the net.
> > + *
> > + * There is an ABA race condition in this sequence:
> > + * 1. the sender sends a packet
> > + * 2. worker calls virtio_transport_recv_pkt(), using the
> > + * sender's net
> > + * 3. virtio_transport_recv_pkt() uses t->send_pkt() passing the
> > + * sender's net
> > + * 4. virtio_transport_recv_pkt() free's the skb, dropping the
> > + * reference to the socket
> > + * 5. the socket closes, frees its reference to the net
> > + * 6. Finally, the worker for the second t->send_pkt() call
> > + * processes the skb, and uses the now stale net pointer for
> > + * socket lookups.
> > + *
> > + * To prevent this, we acquire a net reference in vsock_loopback_send_pkt()
> > + * and hold it until virtio_transport_recv_pkt() completes.
> > + *
> > + * Additionally, we must grab a reference on the skb before
> > + * calling virtio_transport_recv_pkt() to prevent it from
> > + * freeing the skb before we have a chance to release the net.
> > + */
> > + net_mode = virtio_vsock_skb_net_mode(skb);
> > + net = virtio_vsock_skb_net(skb);
>
> Wait, we are adding those just for loopback (in theory used only for
> testing/debugging)? And only to support virtio_transport_reset_no_sock() use
> case?
Yes, exactly, only loopback + reset_no_sock(). The issue doesn't exist
for vhost-vsock because vhost_vsock holds a net reference, and it
doesn't exist for non-reset_no_sock calls because after looking up the
socket we transfer skb ownership to it, which holds down the skb -> sk ->
net reference chain.
>
> Honestly I don't like this, do we have any alternative?
>
> I'll also try to think something else.
>
> Stefano
I've been thinking about this all morning... maybe
we can do something like this:
```
virtio_transport_recv_pkt(..., struct sock *reply_sk) {... }
virtio_transport_reset_no_sock(..., reply_sk)
{
if (reply_sk)
skb_set_owner_sk_safe(reply, reply_sk)
t->send_pkt(reply);
}
vsock_loopback_work(...)
{
virtio_transport_recv_pkt(..., skb, skb->sk);
}
for other transports:
virtio_transport_recv_pkt(..., skb, NULL);
```
This way 'reply' keeps the sk and sk->net alive even after
virtio_transport_recv_pkt() frees 'skb'. The net won't be released until
after 'reply' is freed back on the other side, removing the race.
It makes semantic sense too... for loopback, we already know which sk
the reply is going back to. For other transports, we don't because
they're across the virt boundary.
WDYT?
I hate to suggest this, but another option might be to just do nothing?
In order for this race to have any real effect, a loopback socket must
send a pkt to a non-existent socket, immediately close(), then the
namespace deleted, a new namespace created with the same pointer
address, and finally a new socket with the same port created in that
namespace, all before the reply RST reaches recv_pkt()... at which point
the newly created socket would wrongfully receive the RST.
Best,
Bobby
^ permalink raw reply
* Re: [PATCH net-next v9 08/14] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
From: Bobby Eshleman @ 2025-11-12 18:36 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <ureyl5b2tneivmlce4fdtmuoxgayfxwgewoypb6oyxeh7ozt3i@chygpr2uvtcp>
On Wed, Nov 12, 2025 at 03:21:39PM +0100, Stefano Garzarella wrote:
> On Tue, Nov 11, 2025 at 10:54:50PM -0800, Bobby Eshleman wrote:
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > Reject setting VSOCK_NET_MODE_LOCAL with -EOPNOTSUPP if a G2H transport
> > is operational. Additionally, reject G2H transport registration if there
> > already exists a namespace in local mode.
> >
> > G2H sockets break in local mode because the G2H transports don't support
> > namespacing yet. The current approach is to coerce packets coming out of
> > G2H transports into VSOCK_NET_MODE_GLOBAL mode, but it is not possible
> > to coerce sockets in the same way because it cannot be deduced which
> > transport will be used by the socket. Specifically, when bound to
> > VMADDR_CID_ANY in a nested VM (both G2H and H2G available), it is not
> > until a packet is received and matched to the bound socket that we
> > assign the transport. This presents a chicken-and-egg problem, because
> > we need the namespace to lookup the socket and resolve the transport,
> > but we need the transport to know how to use the namespace during
> > lookup.
> >
> > For that reason, this patch prevents VSOCK_NET_MODE_LOCAL from being
> > used on systems that support G2H, even nested systems that also have H2G
> > transports.
> >
> > Local mode is blocked based on detecting the presence of G2H devices
> > (when possible, as hyperv is special). This means that a host kernel
> > with G2H support compiled in (or has the module loaded), will still
> > support local mode because there is no G2H (e.g., virtio-vsock) device
> > detected. This enables using the same kernel in the host and in the
> > guest, as we do in kselftest.
> >
> > Systems with only namespace-aware transports (vhost-vsock, loopback) can
> > still use both VSOCK_NET_MODE_GLOBAL and VSOCK_NET_MODE_LOCAL modes as
> > intended.
> >
> > The hyperv transport must be treated specially. Other G2H transports can
> > can report presence of a device using get_local_cid(). When a device is
> > present it returns a valid CID; otherwise, it returns VMADDR_CID_ANY.
> > THe hyperv transport's get_local_cid() always returns VMADDR_CID_ANY,
> > however, even when a device is present.
> >
> > For that reason, this patch adds an always_block_local_mode flag to
> > struct vsock_transport. When set to true, VSOCK_NET_MODE_LOCAL is
> > blocked unconditionally whenever the transport is registered, regardless
> > of device presence. When false, LOCAL mode is only blocked when
> > get_local_cid() indicates a device is present (!= VMADDR_CID_ANY).
> >
> > The hyperv transport sets this flag to true to unconditionally block
> > local mode. Other G2H transports (virtio-vsock, vmci-vsock) leave it
> > false and continue using device detection via get_local_cid() to block
> > local mode.
> >
> > These restrictions can be lifted in a future patch series when G2H
> > transports gain namespace support.
>
> IMO this commit should be before supporting namespaces in any device,
> so we are sure we don't have commits where this can happen.
sgtm!
> >
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> > ---
> > include/net/af_vsock.h | 8 +++++++
> > net/vmw_vsock/af_vsock.c | 45 +++++++++++++++++++++++++++++++++++++---
> > net/vmw_vsock/hyperv_transport.c | 1 +
> > 3 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index cfd121bb5ab7..089c61105dda 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -108,6 +108,14 @@ struct vsock_transport_send_notify_data {
> >
> > struct vsock_transport {
> > struct module *module;
> > + /* If true, block VSOCK_NET_MODE_LOCAL unconditionally when this G2H
> > + * transport is registered. If false, only block LOCAL mode when
> > + * get_local_cid() indicates a device is present (!= VMADDR_CID_ANY).
> > + * Hyperv sets this true because it doesn't offer a callback that
> > + * detects device presence. This only applies to G2H transports; H2G
> > + * transports are unaffected.
> > + */
> > + bool always_block_local_mode;
> >
> > /* Initialize/tear-down socket. */
> > int (*init)(struct vsock_sock *, struct vsock_sock *);
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index c0b5946bdc95..a2da1810b802 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
> > @@ -91,6 +91,11 @@
> > * and locked down by a namespace manager. The default is "global". The mode
> > * is set per-namespace.
> > *
> > + * Note: LOCAL mode is only supported when using namespace-aware transports
> > + * (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
> > + * hyperv-vsock, vmci-vsock) is loaded, attempts to set LOCAL mode will fail
> > + * with EOPNOTSUPP, as these transports do not support per-namespace
> > isolation.
> > + *
> > * The modes affect the allocation and accessibility of CIDs as follows:
> > *
> > * - global - access and allocation are all system-wide
> > @@ -2757,12 +2762,30 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> > if (*lenp >= sizeof(data))
> > return -EINVAL;
> >
> > - if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data)))
> > + if (!strncmp(data, VSOCK_NET_MODE_STR_GLOBAL, sizeof(data))) {
> > mode = VSOCK_NET_MODE_GLOBAL;
> > - else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data)))
> > + } else if (!strncmp(data, VSOCK_NET_MODE_STR_LOCAL, sizeof(data))) {
> > + /* LOCAL mode is not supported when G2H transports
> > + * (virtio-vsock, hyperv, vmci) are active, because
> > + * these transports don't support namespaces. We must
> > + * stay in GLOBAL mode to avoid bind/lookup mismatches.
> > + *
> > + * Check if G2H transport is present and either:
> > + * 1. Has always_block_local_mode set (hyperv), OR
> > + * 2. Has an actual device present (get_local_cid() != VMADDR_CID_ANY)
> > + */
> > + mutex_lock(&vsock_register_mutex);
> > + if (transport_g2h &&
> > + (transport_g2h->always_block_local_mode ||
> > + transport_g2h->get_local_cid() != VMADDR_CID_ANY)) {
>
> This seems almost like a hack. What about adding a new function in the
> transports that tells us whether a device is present or not and implement it
> in all of them?
>
> Or a more specific function to check if the transport can work with local
> mode (e.g. netns_local_aware() or something like that - I'm not great with
> nameming xD)
That sounds good to me, I probably prefer option 2 because I think it'll
be simpler for the hyperv case.
>
> > + mutex_unlock(&vsock_register_mutex);
> > + return -EOPNOTSUPP;
> > + }
> > + mutex_unlock(&vsock_register_mutex);
>
> What happen if the G2H is loaded here, just after we release the mutex?
>
> I suspect there could be a race that we may fix postponing the unlock after
> the vsock_net_write_mode() call.
>
> WDYT?
Oh good eye, yeah I think you are right. Writing the net mode should
definitely be in the critical section.
>
> > mode = VSOCK_NET_MODE_LOCAL;
> > - else
> > + } else {
> > return -EINVAL;
> > + }
> >
> > if (!vsock_net_write_mode(net, mode))
> > return -EPERM;
> > @@ -2909,6 +2932,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> > {
> > const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> > int err = mutex_lock_interruptible(&vsock_register_mutex);
> > + struct net *net;
> >
> > if (err)
> > return err;
> > @@ -2931,6 +2955,21 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> > err = -EBUSY;
> > goto err_busy;
> > }
> > +
> > + /* G2H sockets break in LOCAL mode namespaces because G2H transports
> > + * don't support them yet. Block registering new G2H transports if we
> > + * already have local mode namespaces on the system.
> > + */
> > + rcu_read_lock();
> > + for_each_net_rcu(net) {
> > + if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
> > + rcu_read_unlock();
> > + err = -EOPNOTSUPP;
> > + goto err_busy;
> > + }
> > + }
> > + rcu_read_unlock();
> > +
> > t_g2h = t;
> > }
> >
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index 432fcbbd14d4..ed48dd1ff19b 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -835,6 +835,7 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> >
> > static struct vsock_transport hvs_transport = {
> > .module = THIS_MODULE,
> > + .always_block_local_mode = true,
> >
> > .get_local_cid = hvs_get_local_cid,
> >
> >
> > --
> > 2.47.3
> >
>
^ permalink raw reply
* Re: [PATCH net-next v9 03/14] vsock/virtio: add netns support to virtio transport and virtio common
From: Bobby Eshleman @ 2025-11-12 19:32 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Shuah Khan, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, virtualization, netdev,
linux-kselftest, linux-kernel, kvm, linux-hyperv, Sargun Dhillon,
berrange, Bobby Eshleman
In-Reply-To: <bhc6s7anskmnnrnpp2r3xzjbesadsex24kmyi5tvsgup7c2rfi@arj4iw5ndnr3>
On Wed, Nov 12, 2025 at 06:39:22PM +0100, Stefano Garzarella wrote:
> On Wed, Nov 12, 2025 at 08:13:50AM -0800, Bobby Eshleman wrote:
> > On Wed, Nov 12, 2025 at 03:18:42PM +0100, Stefano Garzarella wrote:
> > > On Tue, Nov 11, 2025 at 10:54:45PM -0800, Bobby Eshleman wrote:
> > > > From: Bobby Eshleman <bobbyeshleman@meta.com>
[...]
> > > If it simplifies, I think we can eventually merge all changes to transports
> > > that depends on virtio_transport_common in a single commit.
> > > IMO is better to have working commits than better split.
> >
> > That would be so much easier. Much of this patch is just me trying to
> > find a way to keep total patch size reasonably small for review... if
> > having them all in one commit is preferred then that makes life easier.
> >
> > The answer to all of the above is that I was just trying to make the
> > virtio_common changes in one place, but not break bisect/build by
> > failing to update the transport-level call sites. So the placeholder
> > values are primarily there to compile.
>
> In theory, they should compile, but they should also properly behave.
>
> BTW I strongly believe that having separate commits is a great thing, but we
> shouldn't take things to extremes and complicate our lives when things are
> too closely related, as in this case.
>
> There is a clear dependency between these patches, so IMO, if the patch
> doesn't become huge, it's better to have everything together. (I mean
> between dependencies with virtio_transport_common).
Sounds good, let's give the combined commit a go, I think the
transport-specific pieces are small enough for it to not balloon?
> What we could perhaps do is have an initial commit where you make the
> changes, but the behavior remains unchanged (continue to use global
> everywhere, as for virtio_transport.c in this patch), and then specific
> commits to just enable support for local/global.
>
> Not sure if it's doable, but I'd like to remove the placeholders if
> possibile. Let's discuss more about it if there are issues.
Sounds good, I'll come back to this thread if the combined commit
approach above balloons. For the combined commit, should the change log
start at "Changes in v10" with any new changes, mention combining +
links to the v9 patches that were combined?
Best,
Bobby
^ permalink raw reply
* [PATCH v12 0/3] Drivers: hv: Introduce new driver - mshv_vtl
From: Naman Jain @ 2025-11-13 4:41 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Michael Kelley, Paolo Bonzini, Steven Rostedt, Ard Biesheuvel
Cc: linux-hyperv, linux-kernel, Nuno Das Neves, Magnus Kulke,
Kees Cook, Nathan Chancellor, Miguel Ojeda, Andrew Morton,
Marc Herbert, Jan Hendrik Farr, Naman Jain, Heiko Carstens,
Uros Bizjak, Sean Christopherson, Christoph Hellwig,
Saurabh Sengar, ALOK TIWARI
Introduce a new mshv_vtl driver to provide an interface for Virtual
Machine Monitor like OpenVMM and its use as OpenHCL paravisor to
control VTL0 (Virtual trust Level).
Expose devices and support IOCTLs for features like VTL creation,
VTL0 memory management, context switch, making hypercalls,
mapping VTL0 address space to VTL2 userspace, getting new VMBus
messages and channel events in VTL2 etc.
OpenVMM : https://openvmm.dev/guide/
Changes since v11:
https://lore.kernel.org/all/20251110050835.1603847-1-namjain@linux.microsoft.com/
Addressed Peter Z.'s comments:
* Removed the use of wrapper function and thus STACK_FRAME_NON_STANDARD_FP
by calling STATIC_CALL_TRAMP_STR() directly from assembly code
(mshv_vtl_asm.S)
* This required some changes in static_call_types.h header file as it could
not be included directly in the assembly code. Add a separate patch
as patch 1 of this series to add this support. Patch 3 depends on
patch 1 and 2 for compilation.
* Added a comment in __mshv_vtl_return_call assembly function to add more
information about the handling of NMIs, PFs, noinstr.
* Minor addition of space in __PASTE macros to fix checkpatch errors.
Thanks Peter for helping on this.
Changes since v10:
https://lore.kernel.org/all/20251029050139.46545-1-namjain@linux.microsoft.com/
* Addressed Peter's comments, by changing EXPORT_SYMBOL_GPL to
EXPORT_SYMBOL_FOR_MODULES.
* Add a dependency on HYPERV_VMBUS for MSHV_VTL after recent changes
to separate out HYPERV_VMBUS config and making it tristate. Without
this change, compilation would fail if HYPERV_VMBUS is 'm' and
'MSHV_VTL' is 'y'.
* Rebased to linux-next tag: next-20251110.
Changes since v9:
https://lore.kernel.org/all/20251017074507.142704-1-namjain@linux.microsoft.com/
* Fixed CR2 restore logic in VTL return assembly code
* Fixed an issue with rbp register clobbering in the wrapper function
mshv_vtl_return_hypercall and marked it with
STACK_FRAME_NON_STANDARD_FP to prevent objtool warning.
This addresses an issue which manifested as a crash in VTL0, as the
rbp register value set up by VTL2 before VTL transition changes in
this wrapper function as part of save restore of the stack pointer.
* Minor checkpatch fix for use of extern function in hv_vtl.c
* Rebased to tag: next-20251028
Changes since v8:
https://lore.kernel.org/all/20251013060353.67326-1-namjain@linux.microsoft.com/
Addressed Sean's comments:
* Removed forcing SIGPENDING, and other minor changes, in
mshv_vtl_ioctl_return_to_lower_vtl after referring
to Sean's earlier changes for xfer_to_guest_mode_handle_work.
* Rebased and resolved merge conflicts, compilation errors on latest
linux-next kernel tip, after Roman's Confidential VM changes,
which merged recently. No functional changes.
https://lore.kernel.org/all/20251008233419.20372-1-romank@linux.microsoft.com/
Changes since v7:
https://lore.kernel.org/all/20250729051436.190703-1-namjain@linux.microsoft.com/
Addressed Peter's concerns. Thanks Peter, Paolo, Sean for valuable inputs.
Discussion- https://lore.kernel.org/all/20250825055208.238729-1-namjain@linux.microsoft.com/
* moved assembly code to arch/x86/
* This prevents the need to export hv_hypercall_pg
* Will make it easier to add support for other architectures in the future
* moved assembly code to a separate .S file (arch/x86/hyperv/mshv_vtl_asm.S)
* Used noinstr for this new function in .S file
* Fixed save/restore logic of callee registers, rbp to fix previous objtool warning
* used static call instead of indirect call
* used asm offsets similar to KVM code in assembly file (arch/x86/hyperv/mshv-asm-offsets.c)
* Removed the usage of STACK_FRAME_NON_STANDARD.
Other changes-
* Changed logic to use xfer_to_guest_mode_handle_work and VIRT_XFER_TO_GUEST_WORK
after recently merged changes.
* Removed Reviewed-by Tags after recent changes.
Changes since v6:
https://lore.kernel.org/all/20250724082547.195235-1-namjain@linux.microsoft.com/
Addressed Michael's comments:
* Corrected MAX_BITMAP_SIZE size - finally
* Added missing __packed to hv_synic_overlay_page_msr
* Fixed typo in comment, added CPU hotplug info in comments
* Reverted to mutex_lock/unlock in mshv_vtl_ioctl_set_poll_file
* Unified mshv_vtl_set_reg and mshv_vtl_get_reg
* Dynamic to static allocation of reg in mshv_vtl_ioctl_(get|set)_regs
* Fixed error handling in mshv_vtl_sint_ioctl_signal_event()
Changes since v5:
https://lore.kernel.org/all/20250611072704.83199-1-namjain@linux.microsoft.com/
Addressed Michael Kelley's suggestions:
* Added "depends on HYPERV_VTL_MODE", removed "depends on HYPERV" in Kconfig
* Removed unused macro MAX_GUEST_MEM_SIZE
* Made macro dependency explicit: MSHV_PG_OFF_CPU_MASK and MSHV_REAL_OFF_SHIFT
* Refactored and corrected how allow_bitmap is used and defined. Removed PAGE_SIZE dependency.
* Added __packed for structure definitions wherever it was missing.
* Moved hv_register_vsm_* union definitions to hvgdk_mini.h, kept mshv_synic_overlay_page_msr
in the driver, renamed it and added a comment. (Nuno)
* Introduced global variables input_vtl_zero and input_vtl_normal and used them everywhere these
were defined locally
* s/"page_to_phys(reg_page) >> HV_HYP_PAGE_SHIFT"/"page_to_hvpfn(reg_page)" in
mshv_vtl_configure_reg_page
* Refactored mshv_vtl_vmbus_isr() to reduce complexity in finding and resetting bits similar to
how vmbus_chan_sched is implemented.
* Used __get_free_page() instead in mshv_vtl_alloc_context()
* Added fallback hv_setup_vmbus_handler(vmbus_isr) in hv_vtl_setup_synic() and in
hv_vtl_remove_synic().
* Maintained symmetry of functions in hv_vtl_remove_synic
* Added a note for explanation of excluding last PFN in the range provided in
mshv_vtl_ioctl_add_vtl0_mem()
* Added comments for hotplug being not supported, wherever cpu_online() was used to check if CPU
is online or not.
* Added a check for input.cpu to make sure it's less than nr_cpu_ids in
mshv_vtl_ioctl_set_poll_file()
* Removed switch-case and implemented static tables in mshv_vtl_(get|set)_reg for reducing LOC
* Simplified mshv_vtl_ioctl_(get|set)_regs to process one register at a time, and fixed earlier
bug with array of registers processing.
* Used hv_result_to_errno() in mshv_vtl_sint_ioctl_signal_event()
* Added a READ_ONCE() while reading old_eventfd in mshv_vtl_sint_ioctl_set_eventfd()
* Renamed mshv_vtl_hvcall and mshv_vtl_hvcall_setup to remove ambiguity
* Took care of latest mm patches regarding PFN_DEV, pfn_t deprecation
* Few other minor changes while reorganizing code.
Addressed Markus Elfring's suggestions:
* Used guard(mutex) for better mutex handling.
Changes since v4:
https://lore.kernel.org/all/20250610052435.1660967-1-namjain@linux.microsoft.com/
* Fixed warnings from kernel test robot for missing export.h when the
kernel is compiled with W=1 option.
Some recent changes in kernel flags these warnings and that's why it
was not seen in previous runs. Warnings in other Hyper-V drivers
will be fixed separately.
* No functional changes
Changes since v3:
https://lore.kernel.org/all/20250519045642.50609-1-namjain@linux.microsoft.com/
Addressed Stanislav's, Nuno's comments.
* Change data types for different variables, excluding the ones in uapi headers
* Added comment for the need of HUGEPAGES config in Kconfig.
* generalized new IOCTL names by removing VTL in their name.
* Rebased and added Saurabh's Reviewed-by tag
Changes since v2:
https://lore.kernel.org/all/20250512140432.2387503-1-namjain@linux.microsoft.com/
* Removed CONFIG_OF dependency (addressed Saurabh's comments)
* Fixed typo in "allow_map_intialized" variable name
Changes since v1:
https://lore.kernel.org/all/20250506084937.624680-1-namjain@linux.microsoft.com/
Addressed Saurabh's comments:
* Split the patch in 2 to keep export symbols separate
* Make MSHV_VTL module tristate and fixed compilation warning that would come when HYPERV is
compiled as a module.
* Remove the use of ref_count
* Split functionality of mshv_vtl_ioctl_get_set_regs to different functions
mshv_vtl_ioctl_(get|set)_regs as it actually make things simpler
* Fixed use of copy_from_user in atomic context in mshv_vtl_hvcall_call.
Added ToDo comment for info.
* Added extra code to free memory for vtl in error scenarios in mshv_ioctl_create_vtl()
Addressed Alok's comments regarding:
* Additional conditional checks
* corrected typo in HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB case
* empty lines before return statement
* Added/edited comments, variable names, structure field names as suggested to improve
documentation - no functional change here.
Naman Jain (3):
static_call: allow using STATIC_CALL_TRAMP_STR() from assembly
Drivers: hv: Export some symbols for mshv_vtl
Drivers: hv: Introduce mshv_vtl driver
arch/x86/hyperv/Makefile | 10 +-
arch/x86/hyperv/hv_vtl.c | 29 +
arch/x86/hyperv/mshv-asm-offsets.c | 37 +
arch/x86/hyperv/mshv_vtl_asm.S | 113 ++
arch/x86/include/asm/mshyperv.h | 34 +
drivers/hv/Kconfig | 27 +-
drivers/hv/Makefile | 7 +-
drivers/hv/hv.c | 3 +
drivers/hv/hyperv_vmbus.h | 1 +
drivers/hv/mshv_vtl.h | 25 +
drivers/hv/mshv_vtl_main.c | 1392 +++++++++++++++++++++++
drivers/hv/vmbus_drv.c | 4 +-
include/hyperv/hvgdk_mini.h | 106 ++
include/linux/compiler_types.h | 8 +-
include/linux/static_call_types.h | 4 +
include/uapi/linux/mshv.h | 80 ++
tools/include/linux/static_call_types.h | 4 +
17 files changed, 1876 insertions(+), 8 deletions(-)
create mode 100644 arch/x86/hyperv/mshv-asm-offsets.c
create mode 100644 arch/x86/hyperv/mshv_vtl_asm.S
create mode 100644 drivers/hv/mshv_vtl.h
create mode 100644 drivers/hv/mshv_vtl_main.c
base-commit: ab40c92c74c6b0c611c89516794502b3a3173966
--
2.43.0
^ permalink raw reply
* [PATCH v12 1/3] static_call: allow using STATIC_CALL_TRAMP_STR() from assembly
From: Naman Jain @ 2025-11-13 4:41 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Michael Kelley, Paolo Bonzini, Steven Rostedt, Ard Biesheuvel
Cc: linux-hyperv, linux-kernel, Nuno Das Neves, Magnus Kulke,
Kees Cook, Nathan Chancellor, Miguel Ojeda, Andrew Morton,
Marc Herbert, Jan Hendrik Farr, Naman Jain, Heiko Carstens,
Uros Bizjak, Sean Christopherson, Christoph Hellwig,
Saurabh Sengar, ALOK TIWARI
In-Reply-To: <20251113044149.3710877-1-namjain@linux.microsoft.com>
STATIC_CALL_TRAMP_STR() could not be used from .S files because
static_call_types.h was not safe to include in assembly as it pulled in C
types/constructs that are unavailable under __ASSEMBLY__.
Make the header assembly-friendly by adding __ASSEMBLY__ checks and
providing only the minimal definitions needed for assembly, so that it
can be safely included by .S code. This enables emitting the static call
trampoline symbol name via STATIC_CALL_TRAMP_STR() directly in assembly
sources, to be used with 'call' instruction. Also, move a certain
definitions out of __ASSEMBLY__ checks in compiler_types.h to meet
the dependencies.
No functional change for C compilation.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
include/linux/compiler_types.h | 8 ++++----
include/linux/static_call_types.h | 4 ++++
tools/include/linux/static_call_types.h | 4 ++++
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 0a1b9598940d..c46855162a8a 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -11,6 +11,10 @@
#define __has_builtin(x) (0)
#endif
+/* Indirect macros required for expanded argument pasting, eg. __LINE__. */
+#define ___PASTE(a, b) a##b
+#define __PASTE(a, b) ___PASTE(a, b)
+
#ifndef __ASSEMBLY__
/*
@@ -79,10 +83,6 @@ static inline void __chk_io_ptr(const volatile void __iomem *ptr) { }
# define __builtin_warning(x, y...) (1)
#endif /* __CHECKER__ */
-/* Indirect macros required for expanded argument pasting, eg. __LINE__. */
-#define ___PASTE(a,b) a##b
-#define __PASTE(a,b) ___PASTE(a,b)
-
#ifdef __KERNEL__
/* Attributes */
diff --git a/include/linux/static_call_types.h b/include/linux/static_call_types.h
index 5a00b8b2cf9f..cfb6ddeb292b 100644
--- a/include/linux/static_call_types.h
+++ b/include/linux/static_call_types.h
@@ -25,6 +25,8 @@
#define STATIC_CALL_SITE_INIT 2UL /* init section */
#define STATIC_CALL_SITE_FLAGS 3UL
+#ifndef __ASSEMBLY__
+
/*
* The static call site table needs to be created by external tooling (objtool
* or a compiler plugin).
@@ -100,4 +102,6 @@ struct static_call_key {
#endif /* CONFIG_HAVE_STATIC_CALL */
+#endif /* __ASSEMBLY__ */
+
#endif /* _STATIC_CALL_TYPES_H */
diff --git a/tools/include/linux/static_call_types.h b/tools/include/linux/static_call_types.h
index 5a00b8b2cf9f..cfb6ddeb292b 100644
--- a/tools/include/linux/static_call_types.h
+++ b/tools/include/linux/static_call_types.h
@@ -25,6 +25,8 @@
#define STATIC_CALL_SITE_INIT 2UL /* init section */
#define STATIC_CALL_SITE_FLAGS 3UL
+#ifndef __ASSEMBLY__
+
/*
* The static call site table needs to be created by external tooling (objtool
* or a compiler plugin).
@@ -100,4 +102,6 @@ struct static_call_key {
#endif /* CONFIG_HAVE_STATIC_CALL */
+#endif /* __ASSEMBLY__ */
+
#endif /* _STATIC_CALL_TYPES_H */
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox