Re: [PATCH v3 for 9.1 0/6] virtio, vhost: Add VIRTIO_F_NOTIFICATION_DATA support
Mon, Mar 18, 2024 at 12:22:02PM CET, jonah.pal...@oracle.com wrote: > > >On 3/16/24 11:45 AM, Jiri Pirko wrote: >> Fri, Mar 15, 2024 at 05:55:51PM CET, jonah.pal...@oracle.com wrote: >> > The goal of these patches are to add support to a variety of virtio and >> > vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport feature. This >> > feature indicates that a driver will pass extra data (instead of just a >> > virtqueue's index) when notifying the corresponding device. >> > >> > The data passed in by the driver when this feature is enabled varies in >> > format depending on if the device is using a split or packed virtqueue >> > layout: >> > >> > Split VQ >> > - Upper 16 bits: shadow_avail_idx >> > - Lower 16 bits: virtqueue index >> > >> > Packed VQ >> > - Upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx >> > - Lower 16 bits: virtqueue index >> > >> > Also, due to the limitations of ioeventfd not being able to carry the >> > extra provided by the driver, having both VIRTIO_F_NOTIFICATION_DATA >> > feature and ioeventfd enabled is a functional mismatch. The user must >> > explicitly disable ioeventfd for the device in the Qemu arguments when >> > using this feature, else the device will fail to complete realization. >> > >> > For example, a device must explicitly enable notification_data as well >> > as disable ioeventfd: >> > >> > -device virtio-scsi-pci,...,ioeventfd=off,notification_data=on >> > >> > A significant aspect of this effort has been to maintain compatibility >> > across different backends. As such, the feature is offered by backend >> > devices only when supported, with fallback mechanisms where backend >> > support is absent. >> > >> > v3: Validate VQ idx via. virtio_queue_get_num() (pci, mmio, ccw) >> > Rename virtio_queue_set_shadow_avail_data >> > Only pass in upper 16 bits of 32-bit extra data (was redundant) >> > Make notification compatibility check function static >> > Drop tags on patches 1/6, 3/6, and 4/6 >> > >> > v2: Don't disable ioeventfd by default, user must disable it >> > Drop tags on patch 2/6 >> > >> > Jonah Palmer (6): >> > virtio/virtio-pci: Handle extra notification data >> > virtio: Prevent creation of device using notification-data with ioeventfd >> > virtio-mmio: Handle extra notification data >> > virtio-ccw: Handle extra notification data >> > vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits >> > virtio: Add VIRTIO_F_NOTIFICATION_DATA property definition >> >> Jonah, do you have kernel patches to add this feature as well? >> >> Thanks! > >Hi Jiri! I think there are already kernel patches for >VIRTIO_F_NOTIFICATION_DATA, unless you're referring to something more >specific that wasn't included in these patches: > >[1]: virtio: add VIRTIO_F_NOTIFICATION_DATA feature support >https://lore.kernel.org/lkml/20230324195029.2410503-1-vik...@daynix.com/ > >[2]: virtio-vdpa: add VIRTIO_F_NOTIFICATION_DATA feature support >https://lore.kernel.org/lkml/20230413081855.36643-3-alvaro.ka...@solid-run.com/ I missed this. Thx! > >Jonah
Re: [PATCH v3 for 9.1 0/6] virtio, vhost: Add VIRTIO_F_NOTIFICATION_DATA support
Fri, Mar 15, 2024 at 05:55:51PM CET, jonah.pal...@oracle.com wrote: >The goal of these patches are to add support to a variety of virtio and >vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport feature. This >feature indicates that a driver will pass extra data (instead of just a >virtqueue's index) when notifying the corresponding device. > >The data passed in by the driver when this feature is enabled varies in >format depending on if the device is using a split or packed virtqueue >layout: > > Split VQ > - Upper 16 bits: shadow_avail_idx > - Lower 16 bits: virtqueue index > > Packed VQ > - Upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx > - Lower 16 bits: virtqueue index > >Also, due to the limitations of ioeventfd not being able to carry the >extra provided by the driver, having both VIRTIO_F_NOTIFICATION_DATA >feature and ioeventfd enabled is a functional mismatch. The user must >explicitly disable ioeventfd for the device in the Qemu arguments when >using this feature, else the device will fail to complete realization. > >For example, a device must explicitly enable notification_data as well >as disable ioeventfd: > >-device virtio-scsi-pci,...,ioeventfd=off,notification_data=on > >A significant aspect of this effort has been to maintain compatibility >across different backends. As such, the feature is offered by backend >devices only when supported, with fallback mechanisms where backend >support is absent. > >v3: Validate VQ idx via. virtio_queue_get_num() (pci, mmio, ccw) >Rename virtio_queue_set_shadow_avail_data >Only pass in upper 16 bits of 32-bit extra data (was redundant) >Make notification compatibility check function static >Drop tags on patches 1/6, 3/6, and 4/6 > >v2: Don't disable ioeventfd by default, user must disable it >Drop tags on patch 2/6 > >Jonah Palmer (6): > virtio/virtio-pci: Handle extra notification data > virtio: Prevent creation of device using notification-data with ioeventfd > virtio-mmio: Handle extra notification data > virtio-ccw: Handle extra notification data > vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits > virtio: Add VIRTIO_F_NOTIFICATION_DATA property definition Jonah, do you have kernel patches to add this feature as well? Thanks!
Re: device compatibility interface for live migration with assigned devices
Wed, Aug 05, 2020 at 04:41:54AM CEST, jasow...@redhat.com wrote: > >On 2020/8/5 上午10:16, Yan Zhao wrote: >> On Wed, Aug 05, 2020 at 10:22:15AM +0800, Jason Wang wrote: >> > On 2020/8/5 上午12:35, Cornelia Huck wrote: >> > > [sorry about not chiming in earlier] >> > > >> > > On Wed, 29 Jul 2020 16:05:03 +0800 >> > > Yan Zhao wrote: >> > > >> > > > On Mon, Jul 27, 2020 at 04:23:21PM -0600, Alex Williamson wrote: >> > > (...) >> > > >> > > > > Based on the feedback we've received, the previously proposed >> > > > > interface >> > > > > is not viable. I think there's agreement that the user needs to be >> > > > > able to parse and interpret the version information. Using json >> > > > > seems >> > > > > viable, but I don't know if it's the best option. Is there any >> > > > > precedent of markup strings returned via sysfs we could follow? >> > > I don't think encoding complex information in a sysfs file is a viable >> > > approach. Quoting Documentation/filesystems/sysfs.rst: >> > > >> > > "Attributes should be ASCII text files, preferably with only one value >> > > per file. It is noted that it may not be efficient to contain only one >> > > value per file, so it is socially acceptable to express an array of >> > > values of the same type. >> > > Mixing types, expressing multiple lines of data, and doing fancy >> > > formatting of data is heavily frowned upon." >> > > >> > > Even though this is an older file, I think these restrictions still >> > > apply. >> > >> > +1, that's another reason why devlink(netlink) is better. >> > >> hi Jason, >> do you have any materials or sample code about devlink, so we can have a good >> study of it? >> I found some kernel docs about it but my preliminary study didn't show me the >> advantage of devlink. > > >CC Jiri and Parav for a better answer for this. > >My understanding is that the following advantages are obvious (as I replied >in another thread): > >- existing users (NIC, crypto, SCSI, ib), mature and stable >- much better error reporting (ext_ack other than string or errno) >- namespace aware >- do not couple with kobject Jason, what is your use case? > >Thanks > > >> >> Thanks >> Yan >> >
Re: device compatibility interface for live migration with assigned devices
Wed, Aug 05, 2020 at 11:33:38AM CEST, yan.y.z...@intel.com wrote: >On Wed, Aug 05, 2020 at 04:02:48PM +0800, Jason Wang wrote: >> >> On 2020/8/5 下午3:56, Jiri Pirko wrote: >> > Wed, Aug 05, 2020 at 04:41:54AM CEST, jasow...@redhat.com wrote: >> > > On 2020/8/5 上午10:16, Yan Zhao wrote: >> > > > On Wed, Aug 05, 2020 at 10:22:15AM +0800, Jason Wang wrote: >> > > > > On 2020/8/5 上午12:35, Cornelia Huck wrote: >> > > > > > [sorry about not chiming in earlier] >> > > > > > >> > > > > > On Wed, 29 Jul 2020 16:05:03 +0800 >> > > > > > Yan Zhao wrote: >> > > > > > >> > > > > > > On Mon, Jul 27, 2020 at 04:23:21PM -0600, Alex Williamson wrote: >> > > > > > (...) >> > > > > > >> > > > > > > > Based on the feedback we've received, the previously proposed >> > > > > > > > interface >> > > > > > > > is not viable. I think there's agreement that the user needs >> > > > > > > > to be >> > > > > > > > able to parse and interpret the version information. Using >> > > > > > > > json seems >> > > > > > > > viable, but I don't know if it's the best option. Is there any >> > > > > > > > precedent of markup strings returned via sysfs we could follow? >> > > > > > I don't think encoding complex information in a sysfs file is a >> > > > > > viable >> > > > > > approach. Quoting Documentation/filesystems/sysfs.rst: >> > > > > > >> > > > > > "Attributes should be ASCII text files, preferably with only one >> > > > > > value >> > > > > > per file. It is noted that it may not be efficient to contain only >> > > > > > one >> > > > > > value per file, so it is socially acceptable to express an array of >> > > > > > values of the same type. >> > > > > > Mixing types, expressing multiple lines of data, and doing fancy >> > > > > > formatting of data is heavily frowned upon." >> > > > > > >> > > > > > Even though this is an older file, I think these restrictions still >> > > > > > apply. >> > > > > +1, that's another reason why devlink(netlink) is better. >> > > > > >> > > > hi Jason, >> > > > do you have any materials or sample code about devlink, so we can have >> > > > a good >> > > > study of it? >> > > > I found some kernel docs about it but my preliminary study didn't show >> > > > me the >> > > > advantage of devlink. >> > > >> > > CC Jiri and Parav for a better answer for this. >> > > >> > > My understanding is that the following advantages are obvious (as I >> > > replied >> > > in another thread): >> > > >> > > - existing users (NIC, crypto, SCSI, ib), mature and stable >> > > - much better error reporting (ext_ack other than string or errno) >> > > - namespace aware >> > > - do not couple with kobject >> > Jason, what is your use case? >> >> >> I think the use case is to report device compatibility for live migration. >> Yan proposed a simple sysfs based migration version first, but it looks not >> sufficient and something based on JSON is discussed. >> >> Yan, can you help to summarize the discussion so far for Jiri as a >> reference? >> >yes. >we are currently defining an device live migration compatibility >interface in order to let user space like openstack and libvirt knows >which two devices are live migration compatible. >currently the devices include mdev (a kernel emulated virtual device) >and physical devices (e.g. a VF of a PCI SRIOV device). > >the attributes we want user space to compare including >common attribues: >device_api: vfio-pci, vfio-ccw... >mdev_type: mdev type of mdev or similar signature for physical device > It specifies a device's hardware capability. e.g. > i915-GVTg_V5_4 means it's of 1/4 of a gen9 Intel graphics > device. >software_version: device driver's version. > in .[.bugfix] scheme, where there is no > compatibility across major versio
Re: hw/net/rocker: Dubious code in tx_consume()
Sat, Feb 15, 2020 at 02:15:22PM CET, phi...@redhat.com wrote:
>Hi Jiri,
>
>I am trying to understand this code Scott Feldman added in commit
>dc488f88806:
>
> 157 static int tx_consume(Rocker *r, DescInfo *info)
> 158 {
> ...
> 212 if (tlvs[ROCKER_TLV_TX_TSO_MSS]) {
> 213 tx_tso_mss = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_TSO_MSS]);
> 214 }
> ...
> 252 if (iovcnt) {
> 253 /* XXX perform Tx offloads */
> 254 /* XXX silence compiler for now */
> 255 tx_l3_csum_off += tx_tso_mss = tx_tso_hdr_len = 0;
> 256 }
>
>Nobody complained TSO_MSS is not implemented during almost 5 years.
>Can we remove this code?
Yes, you can.
>
>Thanks,
>
>Phil.
>
Re: [Qemu-devel] [PATCH] net: rocker: set limit to DMA buffer size
Wed, Oct 12, 2016 at 11:10:55AM CEST, ppan...@redhat.com wrote: >From: Prasad J Pandit <p...@fedoraproject.org> > >Rocker network switch emulator has test registers to help debug >DMA operations. While testing host DMA access, a buffer address >is written to register 'TEST_DMA_ADDR' and its size is written to >register 'TEST_DMA_SIZE'. When performing TEST_DMA_CTRL_INVERT >test, if DMA buffer size was greater than 'INT_MAX', it leads to >an invalid buffer access. Limit the DMA buffer size to avoid it. > >Reported-by: Huawei PSIRT <ps...@huawei.com> >Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> Reviewed-by: Jiri Pirko <j...@mellanox.com>
[Qemu-devel] [patch qemu] MAINTAINERS: release Scott from being a rocker maintainer
From: Jiri Pirko <j...@mellanox.com> As requested by Scott, removing him. Signed-off-by: Scott Feldman <sfel...@gmail.com> Signed-off-by: Jiri Pirko <j...@mellanox.com> --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 1d0e2c3..5928f22 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -971,7 +971,6 @@ F: hw/net/vmxnet* F: hw/scsi/vmw_pvscsi* Rocker -M: Scott Feldman <sfel...@gmail.com> M: Jiri Pirko <j...@resnulli.us> S: Maintained F: hw/net/rocker/ -- 2.5.5
[Qemu-devel] [patch qemu v2 3/4] rocker: add name field into WorldOps ale let world specify its name
From: Jiri Pirko <j...@mellanox.com>
Also use this in world_name getter function.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
hw/net/rocker/rocker_of_dpa.c | 1 +
hw/net/rocker/rocker_world.c | 7 +--
hw/net/rocker/rocker_world.h | 1 +
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index da3fc54..0a134eb 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -2614,6 +2614,7 @@ RockerOfDpaGroupList
*qmp_query_rocker_of_dpa_groups(const char *name,
}
static WorldOps of_dpa_ops = {
+.name = "ofdpa",
.init = of_dpa_init,
.uninit = of_dpa_uninit,
.ig = of_dpa_ig,
diff --git a/hw/net/rocker/rocker_world.c b/hw/net/rocker/rocker_world.c
index 1ed0fcd..89777e9 100644
--- a/hw/net/rocker/rocker_world.c
+++ b/hw/net/rocker/rocker_world.c
@@ -98,10 +98,5 @@ enum rocker_world_type world_type(World *world)
const char *world_name(World *world)
{
-switch (world->type) {
-case ROCKER_WORLD_TYPE_OF_DPA:
-return "OF_DPA";
-default:
-return "unknown";
-}
+return world->ops->name;
}
diff --git a/hw/net/rocker/rocker_world.h b/hw/net/rocker/rocker_world.h
index 18d277b..58ade47 100644
--- a/hw/net/rocker/rocker_world.h
+++ b/hw/net/rocker/rocker_world.h
@@ -33,6 +33,7 @@ typedef int (world_cmd)(World *world, DescInfo *info,
RockerTlv *cmd_info_tlv);
typedef struct world_ops {
+const char *name;
world_init *init;
world_uninit *uninit;
world_ig *ig;
--
2.4.3
[Qemu-devel] [patch qemu v2 4/4] rocker: allow user to specify rocker world by property
From: Jiri Pirko <j...@mellanox.com>
Add property to specify rocker world. All ports will be assigned to this
world.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
v1->v2:
- goto error path in case world type is not found for name
- move worlds alloc check right after worlds alloc
---
hw/net/rocker/rocker.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 104c097..30f2ce4 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -43,6 +43,7 @@ struct rocker {
/* switch configuration */
char *name; /* switch name */
+char *world_name;/* world name */
uint32_t fp_ports; /* front-panel port count */
NICPeers *fp_ports_peers;
MACAddr fp_start_macaddr;/* front-panel port 0 mac addr */
@@ -1286,6 +1287,18 @@ static void rocker_msix_uninit(Rocker *r)
rocker_msix_vectors_unuse(r, ROCKER_MSIX_VEC_COUNT(r->fp_ports));
}
+static World *rocker_world_type_by_name(Rocker *r, const char *name)
+{
+int i;
+
+for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
+if (strcmp(name, world_name(r->worlds[i])) == 0) {
+return r->worlds[i];
+ }
+}
+return NULL;
+}
+
static int pci_rocker_init(PCIDevice *dev)
{
Rocker *r = to_rocker(dev);
@@ -1297,7 +1310,6 @@ static int pci_rocker_init(PCIDevice *dev)
/* allocate worlds */
r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
-r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
if (!r->worlds[i]) {
@@ -1306,6 +1318,19 @@ static int pci_rocker_init(PCIDevice *dev)
}
}
+if (!r->world_name) {
+r->world_name =
g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
+}
+
+r->world_dflt = rocker_world_type_by_name(r, r->world_name);
+if (!r->world_dflt) {
+fprintf(stderr,
+"rocker: requested world \"%s\" does not exist\n",
+r->world_name);
+err = -EINVAL;
+goto err_world_type_by_name;
+}
+
/* set up memory-mapped region at BAR0 */
memory_region_init_io(>mmio, OBJECT(r), _mmio_ops, r,
@@ -1439,6 +1464,7 @@ err_duplicate:
err_msix_init:
object_unparent(OBJECT(>msix_bar));
object_unparent(OBJECT(>mmio));
+err_world_type_by_name:
err_world_alloc:
for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
if (r->worlds[i]) {
@@ -1510,6 +1536,7 @@ static void rocker_reset(DeviceState *dev)
static Property rocker_properties[] = {
DEFINE_PROP_STRING("name", Rocker, name),
+DEFINE_PROP_STRING("world", Rocker, world_name),
DEFINE_PROP_MACADDR("fp_start_macaddr", Rocker,
fp_start_macaddr),
DEFINE_PROP_UINT64("switch_id", Rocker,
--
2.4.3
[Qemu-devel] [patch qemu v2 2/4] rocker: return -ENOMEM in case of some world alloc fails
From: Jiri Pirko <j...@mellanox.com>
Until now, 0 is returned in this error case. Fix it ro return -ENOMEM.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
v1->v2:
- new patch
---
hw/net/rocker/rocker.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index a1d921d..104c097 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1301,6 +1301,7 @@ static int pci_rocker_init(PCIDevice *dev)
for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
if (!r->worlds[i]) {
+err = -ENOMEM;
goto err_world_alloc;
}
}
--
2.4.3
[Qemu-devel] [patch qemu v2 1/4] rocker: forbid to change world type
From: Jiri Pirko <j...@mellanox.com>
Port to world assignment should be permitted only by qemu user. Driver
should not be able to do it, so forbid that possibility.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
hw/net/rocker/rocker.c| 8 +++-
hw/net/rocker/rocker_fp.c | 5 +
hw/net/rocker/rocker_fp.h | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index f3e994d..a1d921d 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -400,7 +400,13 @@ static int cmd_set_port_settings(Rocker *r,
if (tlvs[ROCKER_TLV_CMD_PORT_SETTINGS_MODE]) {
mode = rocker_tlv_get_u8(tlvs[ROCKER_TLV_CMD_PORT_SETTINGS_MODE]);
-fp_port_set_world(fp_port, r->worlds[mode]);
+if (mode >= ROCKER_WORLD_TYPE_MAX) {
+return -ROCKER_EINVAL;
+}
+/* We don't support world change. */
+if (!fp_port_check_world(fp_port, r->worlds[mode])) {
+return -ROCKER_EINVAL;
+}
}
if (tlvs[ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING]) {
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index af37fef..0149899 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -186,6 +186,11 @@ void fp_port_set_world(FpPort *port, World *world)
port->world = world;
}
+bool fp_port_check_world(FpPort *port, World *world)
+{
+return port->world == world;
+}
+
bool fp_port_enabled(FpPort *port)
{
return port->enabled;
diff --git a/hw/net/rocker/rocker_fp.h b/hw/net/rocker/rocker_fp.h
index ab80fd8..04592bb 100644
--- a/hw/net/rocker/rocker_fp.h
+++ b/hw/net/rocker/rocker_fp.h
@@ -40,6 +40,7 @@ int fp_port_set_settings(FpPort *port, uint32_t speed,
bool fp_port_from_pport(uint32_t pport, uint32_t *port);
World *fp_port_get_world(FpPort *port);
void fp_port_set_world(FpPort *port, World *world);
+bool fp_port_check_world(FpPort *port, World *world);
bool fp_port_enabled(FpPort *port);
void fp_port_enable(FpPort *port);
void fp_port_disable(FpPort *port);
--
2.4.3
[Qemu-devel] [patch qemu v2 0/4] rocker: prepare for easy addition of other worlds
From: Jiri Pirko <j...@mellanox.com> This patchset does couple of small changes in order to prepare for smooth addition of other worlds, like P4 and BPF. qemu user will be able to request desired rocker world by "world=worldname" property. v1->v2: patch 2/4: - new patch patch 4/4: - goto error path in case world type is not found for name - move worlds alloc check right after worlds alloc Jiri Pirko (4): rocker: forbid to change world type rocker: return -ENOMEM in case of some world alloc fails rocker: add name field into WorldOps ale let world specify its name rocker: allow user to specify rocker world by property hw/net/rocker/rocker.c| 38 -- hw/net/rocker/rocker_fp.c | 5 + hw/net/rocker/rocker_fp.h | 1 + hw/net/rocker/rocker_of_dpa.c | 1 + hw/net/rocker/rocker_world.c | 7 +-- hw/net/rocker/rocker_world.h | 1 + 6 files changed, 45 insertions(+), 8 deletions(-) -- 2.4.3
Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property
Thu, Feb 25, 2016 at 12:31:58PM CET, stefa...@gmail.com wrote:
>On Mon, Feb 22, 2016 at 07:06:34PM +0100, Jiri Pirko wrote:
>> Mon, Feb 22, 2016 at 06:51:50PM CET, stefa...@gmail.com wrote:
>> >On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
>> >> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
>> >> /* allocate worlds */
>> >>
>> >> r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
>> >> -r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
>> >> +
>> >> +if (!r->world_name) {
>> >> +r->world_name =
>> >> g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
>> >> +}
>> >> +
>> >> +r->world_dflt = rocker_world_type_by_name(r, r->world_name);
>> >> +if (!r->world_dflt) {
>> >> +fprintf(stderr,
>> >> +"rocker: requested world \"%s\" does not exist\n",
>> >> +r->world_name);
>> >> +return -EINVAL;
>> >> +}
>> >
>> >world_name is leaked here. Please use goto to run the appropriate
>> >cleanup code instead of returning directly.
>>
>> I did the same what is done with "r->name = g_strdup(ROCKER)"
>>
>> I assumed since this is a property, the caller core will take care of
>> that.
>
>You are right, the string properties will be freed by qdev property
>release.
>
>The return statement added by this patch still leaks
>r->worlds[ROCKER_WORLD_TYPE_OF_DPA] so goto err_world_alloc is needed.
>
>(pci_rocker_uninit() is not called if pci_rocker_init() fails.)
Will fix and send v2. Thanks.
Re: [Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property
Mon, Feb 22, 2016 at 06:51:50PM CET, stefa...@gmail.com wrote:
>On Fri, Feb 19, 2016 at 11:06:43AM +0100, Jiri Pirko wrote:
>> @@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
>> /* allocate worlds */
>>
>> r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
>> -r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
>> +
>> +if (!r->world_name) {
>> +r->world_name =
>> g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
>> +}
>> +
>> +r->world_dflt = rocker_world_type_by_name(r, r->world_name);
>> +if (!r->world_dflt) {
>> +fprintf(stderr,
>> +"rocker: requested world \"%s\" does not exist\n",
>> +r->world_name);
>> +return -EINVAL;
>> +}
>
>world_name is leaked here. Please use goto to run the appropriate
>cleanup code instead of returning directly.
I did the same what is done with "r->name = g_strdup(ROCKER)"
I assumed since this is a property, the caller core will take care of
that.
[Qemu-devel] [patch qemu 2/3] rocker: add name field into WorldOps ale let world specify its name
From: Jiri Pirko <j...@mellanox.com>
Also use this in world_name getter function.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
hw/net/rocker/rocker_of_dpa.c | 1 +
hw/net/rocker/rocker_world.c | 7 +--
hw/net/rocker/rocker_world.h | 1 +
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index da3fc54..0a134eb 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -2614,6 +2614,7 @@ RockerOfDpaGroupList
*qmp_query_rocker_of_dpa_groups(const char *name,
}
static WorldOps of_dpa_ops = {
+.name = "ofdpa",
.init = of_dpa_init,
.uninit = of_dpa_uninit,
.ig = of_dpa_ig,
diff --git a/hw/net/rocker/rocker_world.c b/hw/net/rocker/rocker_world.c
index 1ed0fcd..89777e9 100644
--- a/hw/net/rocker/rocker_world.c
+++ b/hw/net/rocker/rocker_world.c
@@ -98,10 +98,5 @@ enum rocker_world_type world_type(World *world)
const char *world_name(World *world)
{
-switch (world->type) {
-case ROCKER_WORLD_TYPE_OF_DPA:
-return "OF_DPA";
-default:
-return "unknown";
-}
+return world->ops->name;
}
diff --git a/hw/net/rocker/rocker_world.h b/hw/net/rocker/rocker_world.h
index 18d277b..58ade47 100644
--- a/hw/net/rocker/rocker_world.h
+++ b/hw/net/rocker/rocker_world.h
@@ -33,6 +33,7 @@ typedef int (world_cmd)(World *world, DescInfo *info,
RockerTlv *cmd_info_tlv);
typedef struct world_ops {
+const char *name;
world_init *init;
world_uninit *uninit;
world_ig *ig;
--
2.4.3
[Qemu-devel] [patch qemu 0/3] rocker: prepare for easy addition of other worlds
From: Jiri Pirko <j...@mellanox.com> This patchset does couple of small changes in order to prepare for smooth addition of other worlds, like P4 and BPF. qemu user will be able to request desired rocker world by "world=worldname" property. Jiri Pirko (3): rocker: forbid to change world type rocker: add name field into WorldOps ale let world specify its name rocker: allow user to specify rocker world by property hw/net/rocker/rocker.c| 35 +-- hw/net/rocker/rocker_fp.c | 5 + hw/net/rocker/rocker_fp.h | 1 + hw/net/rocker/rocker_of_dpa.c | 1 + hw/net/rocker/rocker_world.c | 7 +-- hw/net/rocker/rocker_world.h | 1 + 6 files changed, 42 insertions(+), 8 deletions(-) -- 2.4.3
[Qemu-devel] [patch qemu 3/3] rocker: allow user to specify rocker world by property
From: Jiri Pirko <j...@mellanox.com>
Add property to specify rocker world. All ports will be assigned to this
world.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
hw/net/rocker/rocker.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index a1d921d..d0bdfcb 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -43,6 +43,7 @@ struct rocker {
/* switch configuration */
char *name; /* switch name */
+char *world_name;/* world name */
uint32_t fp_ports; /* front-panel port count */
NICPeers *fp_ports_peers;
MACAddr fp_start_macaddr;/* front-panel port 0 mac addr */
@@ -1286,6 +1287,18 @@ static void rocker_msix_uninit(Rocker *r)
rocker_msix_vectors_unuse(r, ROCKER_MSIX_VEC_COUNT(r->fp_ports));
}
+static World *rocker_world_type_by_name(Rocker *r, const char *name)
+{
+int i;
+
+for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
+if (strcmp(name, world_name(r->worlds[i])) == 0) {
+return r->worlds[i];
+ }
+}
+return NULL;
+}
+
static int pci_rocker_init(PCIDevice *dev)
{
Rocker *r = to_rocker(dev);
@@ -1297,7 +1310,18 @@ static int pci_rocker_init(PCIDevice *dev)
/* allocate worlds */
r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
-r->world_dflt = r->worlds[ROCKER_WORLD_TYPE_OF_DPA];
+
+if (!r->world_name) {
+r->world_name =
g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
+}
+
+r->world_dflt = rocker_world_type_by_name(r, r->world_name);
+if (!r->world_dflt) {
+fprintf(stderr,
+"rocker: requested world \"%s\" does not exist\n",
+r->world_name);
+return -EINVAL;
+}
for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
if (!r->worlds[i]) {
@@ -1509,6 +1533,7 @@ static void rocker_reset(DeviceState *dev)
static Property rocker_properties[] = {
DEFINE_PROP_STRING("name", Rocker, name),
+DEFINE_PROP_STRING("world", Rocker, world_name),
DEFINE_PROP_MACADDR("fp_start_macaddr", Rocker,
fp_start_macaddr),
DEFINE_PROP_UINT64("switch_id", Rocker,
--
2.4.3
[Qemu-devel] [patch qemu 1/3] rocker: forbid to change world type
From: Jiri Pirko <j...@mellanox.com>
Port to world assignment should be permitted only by qemu user. Driver
should not be able to do it, so forbid that possibility.
Signed-off-by: Jiri Pirko <j...@mellanox.com>
---
hw/net/rocker/rocker.c| 8 +++-
hw/net/rocker/rocker_fp.c | 5 +
hw/net/rocker/rocker_fp.h | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index f3e994d..a1d921d 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -400,7 +400,13 @@ static int cmd_set_port_settings(Rocker *r,
if (tlvs[ROCKER_TLV_CMD_PORT_SETTINGS_MODE]) {
mode = rocker_tlv_get_u8(tlvs[ROCKER_TLV_CMD_PORT_SETTINGS_MODE]);
-fp_port_set_world(fp_port, r->worlds[mode]);
+if (mode >= ROCKER_WORLD_TYPE_MAX) {
+return -ROCKER_EINVAL;
+}
+/* We don't support world change. */
+if (!fp_port_check_world(fp_port, r->worlds[mode])) {
+return -ROCKER_EINVAL;
+}
}
if (tlvs[ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING]) {
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index af37fef..0149899 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -186,6 +186,11 @@ void fp_port_set_world(FpPort *port, World *world)
port->world = world;
}
+bool fp_port_check_world(FpPort *port, World *world)
+{
+return port->world == world;
+}
+
bool fp_port_enabled(FpPort *port)
{
return port->enabled;
diff --git a/hw/net/rocker/rocker_fp.h b/hw/net/rocker/rocker_fp.h
index ab80fd8..04592bb 100644
--- a/hw/net/rocker/rocker_fp.h
+++ b/hw/net/rocker/rocker_fp.h
@@ -40,6 +40,7 @@ int fp_port_set_settings(FpPort *port, uint32_t speed,
bool fp_port_from_pport(uint32_t pport, uint32_t *port);
World *fp_port_get_world(FpPort *port);
void fp_port_set_world(FpPort *port, World *world);
+bool fp_port_check_world(FpPort *port, World *world);
bool fp_port_enabled(FpPort *port);
void fp_port_enable(FpPort *port);
void fp_port_disable(FpPort *port);
--
2.4.3
Re: [Qemu-devel] [PATCH] net: rocker: fix an incorrect array bounds check
Tue, Dec 22, 2015 at 02:07:01PM CET, ppan...@redhat.com wrote: > Hello Scott, Jiri > >A stack overflow issue was reported by Mr Qinghao Tang, CC'd here. It occurs >while processing transmit(tx) descriptors in tx_consume() routine. If a >descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX=16) packet >fragments, the processing loop suffers an off-by-one error. Thus leading to >OOB memory access and leakage of host memory. > >Please see below a proposed patch to fix this issue. Does it look okay? > >=== >From f3461d8098a0572786f5a2d7a492863090c73134 Mon Sep 17 00:00:00 2001 >From: Prasad J Pandit <p...@fedoraproject.org> >Date: Tue, 22 Dec 2015 18:21:00 +0530 >Subject: [PATCH] net: rocker: fix an incorrect array bounds check > >While processing transmit(tx) descriptors in 'tx_consume' routine >the switch emulator suffers from an off-by-one error, if a >descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX) >fragments. Fix an incorrect bounds check to avoid it. > >Reported-by: Qinghao Tang <luodalon...@gmail.com> >Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> Reviewed-by: Jiri Pirko <j...@mellanox.com>
Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense
Thu, Sep 24, 2015 at 06:18:43PM CEST, arm...@redhat.com wrote: >Michael, could you take this one through trivial? Assuming Scott and >Jiri don't mind, and with s/patchas/patch as/ in the commit message. I don't mind :)
Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense
Mon, Sep 14, 2015 at 01:52:23PM CEST, arm...@redhat.com wrote: >g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, >for two reasons. One, it catches multiplication overflowing size_t. >Two, it returns T * rather than void *, which lets the compiler catch >more type errors. > >This commit only touches allocations with size arguments of the form >sizeof(T). Same Coccinelle semantic patchas in commit b45c03f. ^ typo :) Other than that: Acked-by: Jiri Pirko <j...@resnulli.us>
Re: [Qemu-devel] [PATCH] rocker: Use g_new() & friends where that makes obvious sense
Mon, Sep 14, 2015 at 05:55:40PM CEST, ebl...@redhat.com wrote: >On 09/14/2015 05:57 AM, Jiri Pirko wrote: >> Mon, Sep 14, 2015 at 01:52:23PM CEST, arm...@redhat.com wrote: >>> g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, >>> for two reasons. One, it catches multiplication overflowing size_t. >>> Two, it returns T * rather than void *, which lets the compiler catch >>> more type errors. >>> >>> This commit only touches allocations with size arguments of the form >>> sizeof(T). Same Coccinelle semantic patchas in commit b45c03f. >> >>^ typo :) >> > >This typo is copy-pasted into ALL of your recent g_new() cleanups. Since >you did scattershot threads across multiple maintainers rather than one >big thread, it may be a bit harder to plug all the instances before they >get pulled through the various trees. > >> >> Other than that: >> >> Acked-by: Jiri Pirko <j...@resnulli.us> > >Reviewed-by: Eric Blake <ebl...@redhat.com> > >[In qemu, we tend to use 'Reviewed-by' for "I've inspected the code and >agree it correctly does what the commit message claims", and the weaker >'Acked-by' for "I agree with the fix as documented in the commit message >but didn't inspect the code to ensure that they match"] Reviewed-by: Jiri Pirko <j...@mellanox.com>
Re: [Qemu-devel] [PATCH] rocker: fix clang compiler errors
Sat, Mar 07, 2015 at 01:06:24AM CET, dsah...@gmail.com wrote: Consolidate all forward typedef declarations to rocker.h. Signed-off-by: David Ahern dsah...@gmail.com Acked-by: Scott Feldman sfel...@gmail.com Thanks for taking care of this David. Acked-by: Jiri Pirko j...@resnulli.us
[Qemu-devel] [patch qemu] rocker: fix 32bit build
For printf format of uint64_t and size_t use TARGET_FMT_plx and %zu
Signed-off-by: Jiri Pirko j...@resnulli.us
---
hw/net/rocker/rocker.c | 32 +---
hw/net/rocker/rocker_desc.c | 6 +++---
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 672cf5a..4105275 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -774,8 +774,8 @@ static void rocker_io_writel(void *opaque, hwaddr addr,
uint32_t val)
}
break;
default:
-DPRINTF(not implemented dma reg write(l) addr=0x%lx
-val=0x%08x (ring %d, addr=0x%02x)\n,
+DPRINTF(not implemented dma reg write(l) addr=0x TARGET_FMT_plx
+ val=0x%08x (ring %d, addr=0x%02x)\n,
addr, val, index, offset);
break;
}
@@ -816,7 +816,8 @@ static void rocker_io_writel(void *opaque, hwaddr addr,
uint32_t val)
r-lower32 = 0;
break;
default:
-DPRINTF(not implemented write(l) addr=0x%lx val=0x%08x\n, addr, val);
+DPRINTF(not implemented write(l) addr=0x TARGET_FMT_plx
+ val=0x%08x\n, addr, val);
break;
}
}
@@ -834,8 +835,8 @@ static void rocker_io_writeq(void *opaque, hwaddr addr,
uint64_t val)
desc_ring_set_base_addr(r-rings[index], val);
break;
default:
-DPRINTF(not implemented dma reg write(q) addr=0x%lx
-val=0x%016lx (ring %d, offset=0x%02x)\n,
+DPRINTF(not implemented dma reg write(q) addr=0x TARGET_FMT_plx
+ val=0x TARGET_FMT_plx (ring %d, offset=0x%02x)\n,
addr, val, index, offset);
break;
}
@@ -853,8 +854,8 @@ static void rocker_io_writeq(void *opaque, hwaddr addr,
uint64_t val)
rocker_port_phys_enable_write(r, val);
break;
default:
-DPRINTF(not implemented write(q) addr=0x%lx val=0x%016lx\n,
-addr, val);
+DPRINTF(not implemented write(q) addr=0x TARGET_FMT_plx
+ val=0x TARGET_FMT_plx \n, addr, val);
break;
}
}
@@ -945,7 +946,8 @@ static const char *rocker_reg_name(void *opaque, hwaddr
addr)
static void rocker_mmio_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
{
-DPRINTF(Write %s addr %lx, size %u, val %lx\n,
+DPRINTF(Write %s addr TARGET_FMT_plx
+, size %u, val TARGET_FMT_plx \n,
rocker_reg_name(opaque, addr), addr, size, val);
switch (size) {
@@ -1017,8 +1019,8 @@ static uint32_t rocker_io_readl(void *opaque, hwaddr addr)
ret = desc_ring_get_credits(r-rings[index]);
break;
default:
-DPRINTF(not implemented dma reg read(l) addr=0x%lx
-(ring %d, addr=0x%02x)\n, addr, index, offset);
+DPRINTF(not implemented dma reg read(l) addr=0x TARGET_FMT_plx
+ (ring %d, addr=0x%02x)\n, addr, index, offset);
ret = 0;
break;
}
@@ -1072,7 +1074,7 @@ static uint32_t rocker_io_readl(void *opaque, hwaddr addr)
ret = (uint32_t)(r-switch_id 32);
break;
default:
-DPRINTF(not implemented read(l) addr=0x%lx\n, addr);
+DPRINTF(not implemented read(l) addr=0x TARGET_FMT_plx \n, addr);
ret = 0;
break;
}
@@ -1093,8 +1095,8 @@ static uint64_t rocker_io_readq(void *opaque, hwaddr addr)
ret = desc_ring_get_base_addr(r-rings[index]);
break;
default:
-DPRINTF(not implemented dma reg read(q) addr=0x%lx
-(ring %d, addr=0x%02x)\n, addr, index, offset);
+DPRINTF(not implemented dma reg read(q) addr=0x TARGET_FMT_plx
+ (ring %d, addr=0x%02x)\n, addr, index, offset);
ret = 0;
break;
}
@@ -1122,7 +1124,7 @@ static uint64_t rocker_io_readq(void *opaque, hwaddr addr)
ret = r-switch_id;
break;
default:
-DPRINTF(not implemented read(q) addr=0x%lx\n, addr);
+DPRINTF(not implemented read(q) addr=0x TARGET_FMT_plx \n, addr);
ret = 0;
break;
}
@@ -1131,7 +1133,7 @@ static uint64_t rocker_io_readq(void *opaque, hwaddr addr)
static uint64_t rocker_mmio_read(void *opaque, hwaddr addr, unsigned size)
{
-DPRINTF(Read %s addr %lx, size %u\n,
+DPRINTF(Read %s addr TARGET_FMT_plx , size %u\n,
rocker_reg_name(opaque, addr), addr, size);
switch (size) {
diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
index 83c2b98..0a6dfae 100644
--- a/hw/net/rocker/rocker_desc.c
+++ b/hw/net/rocker/rocker_desc.c
@@ -83,7 +83,7 @@ int desc_set_buf(DescInfo *info, size_t tlv_size)
if (tlv_size info-buf_size) {
DPRINTF(ERROR: trying to write more to desc
Re: [Qemu-devel] [PATCH v5 10/10] rocker: timestamp on the debug logs helps correlate with events in the VM
Thu, Jan 22, 2015 at 09:03:59AM CET, sfel...@gmail.com wrote:
From: David Ahern dsah...@gmail.com
Signed-off-by: David Ahern dsah...@gmail.com
Signed-off-by: Scott Feldman sfel...@gmail.com
Signed-off-by: Jiri Pirko j...@resnulli.us
---
hw/net/rocker/rocker.h | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/net/rocker/rocker.h b/hw/net/rocker/rocker.h
index ef77487..5ae8aff 100644
--- a/hw/net/rocker/rocker.h
+++ b/hw/net/rocker/rocker.h
@@ -25,7 +25,16 @@
#if defined(DEBUG_ROCKER)
# define DPRINTF(fmt, ...) \
-do { fprintf(stderr, ROCKER: fmt, ## __VA_ARGS__); } while (0)
+do { \
+struct timeval tv; \
+char timestr[64]; \
+time_t now;\
+gettimeofday(tv, NULL); \
+now = tv.tv_sec; \
+strftime(timestr, sizeof(timestr), %T, localtime(now)); \
+fprintf(stderr, %s.%06ld , timestr, tv.tv_usec); \
+fprintf(stderr, ROCKER: fmt, ## __VA_ARGS__); \
+} while (0)
#else
static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...)
{
--
1.7.10.4
[Qemu-devel] [patch qemu] net: move queue number into NICPeers
It indicates the number of elements in ncs field and makes sense to have
int inside NICPeers. Also in parse_netdev we do not need to access
container and work with NICPeers only.
Signed-off-by: Jiri Pirko j...@resnulli.us
---
hw/core/qdev-properties-system.c | 3 +--
hw/net/virtio-net.c | 2 +-
include/net/net.h| 2 +-
net/net.c| 4 ++--
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 404cf18..3fd592c 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -180,7 +180,6 @@ PropertyInfo qdev_prop_chr = {
static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
{
NICPeers *peers_ptr = (NICPeers *)ptr;
-NICConf *conf = container_of(peers_ptr, NICConf, peers);
NetClientState **ncs = peers_ptr-ncs;
NetClientState *peers[MAX_QUEUE_NUM];
int queues, i = 0;
@@ -219,7 +218,7 @@ static int parse_netdev(DeviceState *dev, const char *str,
void **ptr)
ncs[i]-queue_index = i;
}
-conf-queues = queues;
+peers_ptr-queues = queues;
return 0;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 940a7cf..b4cb277 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1533,7 +1533,7 @@ static void virtio_net_device_realize(DeviceState *dev,
Error **errp)
virtio_init(vdev, virtio-net, VIRTIO_ID_NET, n-config_size);
-n-max_queues = MAX(n-nic_conf.queues, 1);
+n-max_queues = MAX(n-nic_conf.peers.queues, 1);
n-vqs = g_malloc0(sizeof(VirtIONetQueue) * n-max_queues);
n-vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
n-curr_queues = 1;
diff --git a/include/net/net.h b/include/net/net.h
index 8166345..8a1db8a 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -24,13 +24,13 @@ struct MACAddr {
typedef struct NICPeers {
NetClientState *ncs[MAX_QUEUE_NUM];
+int32_t queues;
} NICPeers;
typedef struct NICConf {
MACAddr macaddr;
NICPeers peers;
int32_t bootindex;
-int32_t queues;
} NICConf;
#define DEFINE_NIC_PROPERTIES(_state, _conf)\
diff --git a/net/net.c b/net/net.c
index 0ff2e40..0246818 100644
--- a/net/net.c
+++ b/net/net.c
@@ -233,7 +233,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
{
NetClientState **peers = conf-peers.ncs;
NICState *nic;
-int i, queues = MAX(1, conf-queues);
+int i, queues = MAX(1, conf-peers.queues);
assert(info-type == NET_CLIENT_OPTIONS_KIND_NIC);
assert(info-size = sizeof(NICState));
@@ -346,7 +346,7 @@ void qemu_del_net_client(NetClientState *nc)
void qemu_del_nic(NICState *nic)
{
-int i, queues = MAX(nic-conf-queues, 1);
+int i, queues = MAX(nic-conf-peers.queues, 1);
/* If this is a peer NIC and peer has already been deleted, free it now. */
if (nic-peer_deleted) {
--
1.9.0
[Qemu-devel] [patch qemu] vmxnet3: fix msix vectors unuse
In vmxnet3_cleanup_msix(), there is called msix_vector_unuse() with
VMXNET3_MAX_INTRS. That is not correct since vector of
value VMXNET3_MAX_INTRS was never used. Also all the used vectors
are not un-used. So call vmxnet3_unuse_msix_vectors() instead which
does the correct job.
Signed-off-by: Jiri Pirko j...@resnulli.us
---
hw/net/vmxnet3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 1bb9259..f3be494 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2050,7 +2050,7 @@ vmxnet3_cleanup_msix(VMXNET3State *s)
PCIDevice *d = PCI_DEVICE(s);
if (s-msix_used) {
-msix_vector_unuse(d, VMXNET3_MAX_INTRS);
+vmxnet3_unuse_msix_vectors(s, VMXNET3_MAX_INTRS);
msix_uninit(d, s-msix_bar, s-msix_bar);
}
}
--
1.9.0
