Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- docs/schemas/network.rng | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 56937d6a4e..677ec77724 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -237,9 +237,9 @@ <optional> <ref name="bandwidth"/> </optional> - <optional> - <ref name="vlan"/> - </optional> + <optional> + <ref name="vlan"/> + </optional> </interleave> </element> </zeroOrMore> -- 2.24.1

This is in the data structure and the parse/format functions, and is getting passed all around correctly, it just was omitted from the RNG, which hasn't been noticed because no human is creating <networkport> XML, and so it's never getting validated against the schema. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- docs/schemas/networkport.rng | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/schemas/networkport.rng b/docs/schemas/networkport.rng index 8cfc043a40..ea43c03d41 100644 --- a/docs/schemas/networkport.rng +++ b/docs/schemas/networkport.rng @@ -29,6 +29,9 @@ <optional> <ref name="bandwidth"/> </optional> + <optional> + <ref name="vlan"/> + </optional> <optional> <ref name="plug"/> </optional> -- 2.24.1

Not only was the original error code destroyed in the case of encountering an error during recovery from a failed attach to the bridge (and then *that* error was destroyed by logging a *second* error about the failure to recover - virNetDevBridgeAddPort() already logs an error, so the one about failing to recover was redundant), but if the recovery was successful, the function would then return success to the caller even though it had failed. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c840889968..6395826c69 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3352,14 +3352,13 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm, ret = virNetDevBridgeAddPort(newbridge, olddev->ifname); virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0); if (ret < 0) { + virErrorPtr err; + + virErrorPreserveLast(&err); ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname); virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0); - if (ret < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("unable to recover former state by adding port " - "to bridge %s"), oldbridge); - } - return ret; + virErrorRestore(&err); + return -1; } /* caller will replace entire olddev with newdev in domain nets list */ return 0; -- 2.24.1

When this flag is set for an interface attached to a bridge, traffic to/from the specified interface can only enter/exit the bridge via another attached interface that *doesn't* have the BR_ISOLATED flag set. This can be used to permit guests to communicate with the rest of the network, but not with each other. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/libvirt_private.syms | 2 ++ src/util/virnetdevbridge.c | 46 ++++++++++++++++++++++++++++++++++++++ src/util/virnetdevbridge.h | 9 ++++++++ 3 files changed, 57 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc0449d1d8..5d043041e0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2552,8 +2552,10 @@ virNetDevBridgeFDBDel; virNetDevBridgeGetSTP; virNetDevBridgeGetSTPDelay; virNetDevBridgeGetVlanFiltering; +virNetDevBridgePortGetIsolated; virNetDevBridgePortGetLearning; virNetDevBridgePortGetUnicastFlood; +virNetDevBridgePortSetIsolated; virNetDevBridgePortSetLearning; virNetDevBridgePortSetUnicastFlood; virNetDevBridgeRemovePort; diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 769289ae0b..d15e81daeb 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -311,6 +311,30 @@ virNetDevBridgePortSetUnicastFlood(const char *brname, } +int +virNetDevBridgePortGetIsolated(const char *brname, + const char *ifname, + bool *enable) +{ + unsigned long value; + + if (virNetDevBridgePortGet(brname, ifname, "isolated", &value) < 0) + return -1; + + *enable = !!value; + return 0; +} + + +int +virNetDevBridgePortSetIsolated(const char *brname, + const char *ifname, + bool enable) +{ + return virNetDevBridgePortSet(brname, ifname, "isolated", enable ? 1 : 0); +} + + #else int virNetDevBridgePortGetLearning(const char *brname G_GNUC_UNUSED, @@ -354,6 +378,28 @@ virNetDevBridgePortSetUnicastFlood(const char *brname G_GNUC_UNUSED, _("Unable to set bridge port unicast_flood on this platform")); return -1; } + + +int +virNetDevBridgePortGetIsolated(const char *brname G_GNUC_UNUSED, + const char *ifname G_GNUC_UNUSED, + bool *enable G_GNUC_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to get bridge port isolated on this platform")); + return -1; +} + + +int +virNetDevBridgePortSetIsolated(const char *brname G_GNUC_UNUSED, + const char *ifname G_GNUC_UNUSED, + bool enable G_GNUC_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Unable to set bridge port isolated on this platform")); + return -1; +} #endif diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h index 8137914da8..db4099bf0b 100644 --- a/src/util/virnetdevbridge.h +++ b/src/util/virnetdevbridge.h @@ -73,6 +73,15 @@ int virNetDevBridgePortSetUnicastFlood(const char *brname, const char *ifname, bool enable) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +int virNetDevBridgePortGetIsolated(const char *brname, + const char *ifname, + bool *enable) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + G_GNUC_WARN_UNUSED_RESULT; +int virNetDevBridgePortSetIsolated(const char *brname, + const char *ifname, + bool enable) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; typedef enum { VIR_NETDEVBRIDGE_FDB_FLAG_ROUTER = (1 << 0), -- 2.24.1

This is a very simple thing to parse and format, but needs to be done in 4 places, so two trivial utility functions have been made that can be called from all the higher level parser/formatters: <domain><interface> <domain><interface><actual> (only in domain status) <network> <networkport> Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- docs/schemas/domaincommon.rng | 3 + docs/schemas/network.rng | 3 + docs/schemas/networkcommon.rng | 11 ++++ docs/schemas/networkport.rng | 3 + src/conf/domain_conf.c | 19 ++++++ src/conf/domain_conf.h | 4 ++ src/conf/network_conf.c | 32 ++++++++++ src/conf/network_conf.h | 9 +++ src/conf/virnetworkportdef.c | 3 + src/conf/virnetworkportdef.h | 1 + src/libvirt_private.syms | 1 + tests/networkxml2xmlin/isolated-ports.xml | 7 +++ tests/networkxml2xmlout/isolated-ports.xml | 7 +++ tests/networkxml2xmltest.c | 1 + tests/qemuxml2argvdata/net-isolated-port.xml | 34 ++++++++++ .../net-isolated-port.x86_64-latest.xml | 63 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 17 files changed, 202 insertions(+) create mode 100644 tests/networkxml2xmlin/isolated-ports.xml create mode 100644 tests/networkxml2xmlout/isolated-ports.xml create mode 100644 tests/qemuxml2argvdata/net-isolated-port.xml create mode 100644 tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 29b6b95357..5bb8281a59 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3159,6 +3159,9 @@ <optional> <ref name="vlan"/> </optional> + <optional> + <ref name="portOptions"/> + </optional> <optional> <element name="teaming"> <choice> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 677ec77724..60453225d6 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -332,6 +332,9 @@ <optional> <ref name="vlan"/> </optional> + <optional> + <ref name="portOptions"/> + </optional> <!-- <ip> element --> <zeroOrMore> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index fd1aac6485..c0eeb5f2c5 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -280,4 +280,15 @@ </attribute> </element> </define> + + <define name="portOptions"> + <element name="portOptions"> + <optional> + <attribute name="isolated"> + <ref name="virYesNo"/> + </attribute> + </optional> + </element> + </define> + </grammar> diff --git a/docs/schemas/networkport.rng b/docs/schemas/networkport.rng index ea43c03d41..031c5241f0 100644 --- a/docs/schemas/networkport.rng +++ b/docs/schemas/networkport.rng @@ -32,6 +32,9 @@ <optional> <ref name="vlan"/> </optional> + <optional> + <ref name="portOptions"/> + </optional> <optional> <ref name="plug"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7a0d126784..176550b62f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11480,6 +11480,9 @@ virDomainActualNetDefParseXML(xmlNodePtr node, if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &actual->vlan) < 0) goto error; + if (virNetworkPortOptionsParseXML(ctxt, &actual->isolatedPort) < 0) + goto error; + *def = g_steal_pointer(&actual); ret = 0; error: @@ -12376,6 +12379,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0) + goto error; + cleanup: virDomainActualNetDefFree(actual); virHashFree(filterparams); @@ -25453,6 +25459,7 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf, return -1; if (virNetDevBandwidthFormat(virDomainNetGetActualBandwidth(def), 0, buf) < 0) return -1; + virNetworkPortOptionsFormat(virDomainNetGetActualPortOptionsIsolated(def), buf); return 0; } @@ -25829,6 +25836,7 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; if (virNetDevBandwidthFormat(def->bandwidth, 0, buf) < 0) return -1; + virNetworkPortOptionsFormat(def->isolatedPort, buf); /* ONLY for internal status storage - format the ActualNetDef * as a subelement of <interface> so that no persistent config @@ -29906,6 +29914,17 @@ virDomainNetGetActualVlan(const virDomainNetDef *iface) } +virTristateBool +virDomainNetGetActualPortOptionsIsolated(const virDomainNetDef *iface) +{ + if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && + iface->data.network.actual) { + return iface->data.network.actual->isolatedPort; + } + return iface->isolatedPort; +} + + bool virDomainNetGetActualTrustGuestRxFilters(const virDomainNetDef *iface) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 867a9c7661..cdc4d25700 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -928,6 +928,7 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool isolatedPort; unsigned int class_id; /* class ID for bandwidth 'floor' */ }; @@ -1032,6 +1033,7 @@ struct _virDomainNetDef { virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool isolatedPort; int linkstate; unsigned int mtu; virNetDevCoalescePtr coalesce; @@ -3239,6 +3241,8 @@ const virNetDevBandwidth * virDomainNetGetActualBandwidth(const virDomainNetDef *iface); const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface); bool virDomainNetGetActualTrustGuestRxFilters(const virDomainNetDef *iface); +virTristateBool +virDomainNetGetActualPortOptionsIsolated(const virDomainNetDef *iface); const char *virDomainNetGetModelString(const virDomainNetDef *net); int virDomainNetSetModelString(virDomainNetDefPtr et, const char *model); diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 1f14a964a2..e9cc9bb55a 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1172,6 +1172,26 @@ virNetworkIPDefParseXML(const char *networkName, } +int +virNetworkPortOptionsParseXML(xmlXPathContextPtr ctxt, + virTristateBool *isolatedPort) +{ + g_autofree char *str = NULL; + int tmp = VIR_TRISTATE_BOOL_ABSENT; + + if ((str = virXPathString("string(./portOptions/@isolated)", ctxt))) { + if ((tmp = virTristateBoolTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown port isolated value '%s'"), str); + return -1; + } + } + + *isolatedPort = tmp; + return 0; +} + + static int virNetworkPortGroupParseXML(virPortGroupDefPtr def, xmlNodePtr node, @@ -1725,6 +1745,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0) goto error; + if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0) + goto error; + /* Parse bridge information */ def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt); def->bridgeZone = virXPathString("string(./bridge[1]/@zone)", ctxt); @@ -2331,6 +2354,14 @@ virNetworkIPDefFormat(virBufferPtr buf, return 0; } +void +virNetworkPortOptionsFormat(virTristateBool isolatedPort, + virBufferPtr buf) +{ + if (isolatedPort != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(buf, "<portOptions isolated='%s'/>\n", + virTristateBoolTypeToString(isolatedPort)); +} static int virPortGroupDefFormat(virBufferPtr buf, @@ -2608,6 +2639,7 @@ virNetworkDefFormatBuf(virBufferPtr buf, return -1; if (virNetDevBandwidthFormat(def->bandwidth, 0, buf) < 0) return -1; + virNetworkPortOptionsFormat(def->isolatedPort, buf); for (i = 0; i < def->nips; i++) { if (virNetworkIPDefFormat(buf, &def->ips[i]) < 0) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index d5dd8480db..db7243eef5 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -272,6 +272,7 @@ struct _virNetworkDef { virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool isolatedPort; /* Application-specific custom metadata */ xmlNodePtr metadata; @@ -377,6 +378,14 @@ virNetworkConfigFile(const char *dir, void virNetworkSetBridgeMacAddr(virNetworkDefPtr def); +int +virNetworkPortOptionsParseXML(xmlXPathContextPtr ctxt, + virTristateBool *isolatedPort); + +void +virNetworkPortOptionsFormat(virTristateBool isolatedPort, + virBufferPtr buf); + VIR_ENUM_DECL(virNetworkForward); #define VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE \ diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 28a58ad8f8..a0705a8322 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -161,6 +161,8 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0) return NULL; + if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0) + return NULL; trustGuestRxFilters = virXPathString("string(./rxfilters/@trustGuest)", ctxt); @@ -360,6 +362,7 @@ virNetworkPortDefFormatBuf(virBufferPtr buf, virNetDevBandwidthFormat(def->bandwidth, def->class_id, buf); if (virNetDevVlanFormat(&def->vlan, buf) < 0) return -1; + virNetworkPortOptionsFormat(def->isolatedPort, buf); if (def->trustGuestRxFilters) virBufferAsprintf(buf, "<rxfilters trustGuest='%s'/>\n", virTristateBoolTypeToString(def->trustGuestRxFilters)); diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index f5ba337fc9..78cf2c1ba4 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -60,6 +60,7 @@ struct _virNetworkPortDef { unsigned int class_id; /* class ID for bandwidth 'floor' */ virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ + virTristateBool isolatedPort; int plugtype; /* virNetworkPortPlugType */ union { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5d043041e0..8f3312d0df 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -513,6 +513,7 @@ virDomainNetGetActualBridgeName; virDomainNetGetActualDirectDev; virDomainNetGetActualDirectMode; virDomainNetGetActualHostdev; +virDomainNetGetActualPortOptionsIsolated; virDomainNetGetActualTrustGuestRxFilters; virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; diff --git a/tests/networkxml2xmlin/isolated-ports.xml b/tests/networkxml2xmlin/isolated-ports.xml new file mode 100644 index 0000000000..be21f155b2 --- /dev/null +++ b/tests/networkxml2xmlin/isolated-ports.xml @@ -0,0 +1,7 @@ +<network> + <name>port-isolation-test</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="br0"/> + <forward mode="bridge"/> + <portOptions isolated="yes"/> +</network> diff --git a/tests/networkxml2xmlout/isolated-ports.xml b/tests/networkxml2xmlout/isolated-ports.xml new file mode 100644 index 0000000000..eed4461574 --- /dev/null +++ b/tests/networkxml2xmlout/isolated-ports.xml @@ -0,0 +1,7 @@ +<network> + <name>port-isolation-test</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode='bridge'/> + <bridge name='br0'/> + <portOptions isolated='yes'/> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index f784b90c69..ec679e72ee 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -160,6 +160,7 @@ mymain(void) DO_TEST("metadata"); DO_TEST("set-mtu"); DO_TEST("dnsmasq-options"); + DO_TEST("isolated-ports"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemuxml2argvdata/net-isolated-port.xml b/tests/qemuxml2argvdata/net-isolated-port.xml new file mode 100644 index 0000000000..1d53c0cd6b --- /dev/null +++ b/tests/qemuxml2argvdata/net-isolated-port.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <interface type='network'> + <mac address='52:54:00:d6:c0:0b'/> + <source network='default'/> + <portOptions isolated='yes'/> + <model type='virtio'/> + </interface> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml b/tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml new file mode 100644 index 0000000000..0605d36a87 --- /dev/null +++ b/tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml @@ -0,0 +1,63 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x10'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='2' port='0x11'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0x12'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x2'/> + </controller> + <interface type='network'> + <mac address='52:54:00:d6:c0:0b'/> + <source network='default'/> + <portOptions isolated='yes'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4b1699db7e..073a7e5565 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -462,6 +462,7 @@ mymain(void) DO_TEST("net-virtio-teaming-network", QEMU_CAPS_VIRTIO_NET_FAILOVER, QEMU_CAPS_DEVICE_VFIO_PCI); + DO_TEST_CAPS_LATEST("net-isolated-port"); DO_TEST("net-hostdev", NONE); DO_TEST("net-hostdev-bootorder", NONE); DO_TEST("net-hostdev-vfio", QEMU_CAPS_DEVICE_VFIO_PCI); -- 2.24.1

Similar to the way that the <vlan>, <bandwidth>, and <virtualport> elements and the trustGuestRxFilters attribute in a <network> (or in the appropriate <portgroup> element of a <network> can be applied to a port when it is allocated for a domain's network interface, this patch checks for a configured value of <portOptions isolated="yes|no"/> in either the domain <interface> or in the network, setting isolatedPort in the <networkport> to the first one it finds (the setting from the domain's <interface> is preferred). This, in turn, is passed back to the domain when a port is allocated, so that the domain will use that setting. (One difference from <vlan>, <bandwidth>, <virtualport>, and trustGuestRxFilters, is that all of those can be set in a <portgroup> so that they can be applied only to a subset of interfaces connected to the network. This didn't really make sense for the isolated setting due to the way that it's implemented in Linux - the BR_ISOLATED flag will prevent traffic from passing between two ports that both have BR_ISOLATED set, but traffic can still go between those ports and other ports that *don't* have BR_ISOLATED. (It would be nice if all traffic from a BR_ISOLATED port could be blocked except traffic going to/from a designated egress port or ports, but instead the entire feature is implemented as a single flag. Because of this, it's really only useful if all the ports on a network are isolated, so setting it for a subset has no practical utility.) Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/conf/domain_conf.c | 3 +++ src/network/bridge_driver.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 176550b62f..dd35522370 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30800,6 +30800,7 @@ virDomainNetDefToNetworkPort(virDomainDefPtr dom, if (virNetDevVlanCopy(&port->vlan, &iface->vlan) < 0) return NULL; + port->isolatedPort = iface->isolatedPort; port->trustGuestRxFilters = iface->trustGuestRxFilters; return g_steal_pointer(&port); @@ -30899,6 +30900,7 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDefPtr iface, if (virNetDevVlanCopy(&actual->vlan, &port->vlan) < 0) goto error; + actual->isolatedPort = port->isolatedPort; actual->class_id = port->class_id; actual->trustGuestRxFilters = port->trustGuestRxFilters; @@ -31038,6 +31040,7 @@ virDomainNetDefActualToNetworkPort(virDomainDefPtr dom, if (virNetDevVlanCopy(&port->vlan, &actual->vlan) < 0) return NULL; + port->isolatedPort = actual->isolatedPort; port->class_id = actual->class_id; port->trustGuestRxFilters = actual->trustGuestRxFilters; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 94212eec77..e26c5a4879 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4532,6 +4532,9 @@ networkAllocatePort(virNetworkObjPtr obj, port->trustGuestRxFilters = netdef->trustGuestRxFilters; } + if (port->isolatedPort == VIR_TRISTATE_BOOL_ABSENT) + port->isolatedPort = netdef->isolatedPort; + /* merge virtualports from interface, network, and portgroup to * arrive at actual virtualport to use */ -- 2.24.1

This patch pushes the isolatedPort setting from the <interface> down all the way to the callers of virNetDevBridgeAddPort(), and sets BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after the port has been successfully added to the bridge. Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/bhyve/bhyve_command.c | 1 + src/conf/domain_conf.c | 1 + src/lxc/lxc_process.c | 10 ++++++++++ src/network/bridge_driver.c | 1 + src/qemu/qemu_hotplug.c | 16 ++++++++++++++++ src/qemu/qemu_interface.c | 1 + src/util/virnetdevtap.c | 17 ++++++++++++++++- src/util/virnetdevtap.h | 3 +++ tests/bhyvexml2argvmock.c | 1 + 9 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index a8bfc0aa72..2df7b60115 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -95,6 +95,7 @@ bhyveBuildNetArgStr(virConnectPtr conn, def->uuid, NULL, NULL, 0, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), + virDomainNetGetActualPortOptionsIsolated(net), NULL, 0, NULL, VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { goto cleanup; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dd35522370..30b2a53b83 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31146,6 +31146,7 @@ virDomainNetNotifyActualDevice(virConnectPtr conn, &iface->mac, dom->uuid, virDomainNetGetActualVirtPortProfile(iface), virDomainNetGetActualVlan(iface), + virDomainNetGetActualPortOptionsIsolated(iface), iface->mtu, NULL)); } } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index da6df86834..6851b3e3e2 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -303,6 +303,16 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm, } else { if (virNetDevBridgeAddPort(brname, parentVeth) < 0) return NULL; + + if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES && + virNetDevBridgePortSetIsolated(brname, parentVeth, true) < 0) { + virErrorPtr err; + + virErrorPreserveLast(&err); + ignore_value(virNetDevBridgeRemovePort(brname, parentVeth)); + virErrorRestore(&err); + return NULL; + } } } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e26c5a4879..27d9a24de9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2489,6 +2489,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, if (virNetDevTapCreateInBridgePort(def->bridge, &macTapIfName, &def->mac, NULL, NULL, &tapfd, 1, NULL, NULL, + VIR_TRISTATE_BOOL_NO, NULL, def->mtu, NULL, VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE | VIR_NETDEV_TAP_CREATE_IFUP | diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6395826c69..af892255c7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3350,12 +3350,28 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm, } ret = virNetDevBridgeAddPort(newbridge, olddev->ifname); + if (ret == 0 && + virDomainNetGetActualPortOptionsIsolated(newdev) == VIR_TRISTATE_BOOL_YES) { + + ret = virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true); + if (ret < 0) { + virErrorPtr err; + + virErrorPreserveLast(&err); + ignore_value(virNetDevBridgeRemovePort(newbridge, olddev->ifname)); + virErrorRestore(&err); + } + } virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0); if (ret < 0) { virErrorPtr err; virErrorPreserveLast(&err); ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname); + if (ret == 0 && + virDomainNetGetActualPortOptionsIsolated(olddev) == VIR_TRISTATE_BOOL_YES) { + ignore_value(virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true)); + } virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0); virErrorRestore(&err); return -1; diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index 74d4782599..8a01eecd83 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -568,6 +568,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, def->uuid, tunpath, tapfd, *tapfdSize, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), + virDomainNetGetActualPortOptionsIsolated(net), net->coalesce, 0, NULL, tap_create_flags) < 0) { virDomainAuditNetDevice(def, net, tunpath, false); diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 84d91428e7..7bd30ea0f9 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -505,6 +505,7 @@ virNetDevTapAttachBridge(const char *tapname, const unsigned char *vmuuid, const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, + virTristateBool isolatedPort, unsigned int mtu, unsigned int *actualMTU) { @@ -545,6 +546,16 @@ virNetDevTapAttachBridge(const char *tapname, } else { if (virNetDevBridgeAddPort(brname, tapname) < 0) return -1; + + if (isolatedPort == VIR_TRISTATE_BOOL_YES && + virNetDevBridgePortSetIsolated(brname, tapname, true) < 0) { + virErrorPtr err; + + virErrorPreserveLast(&err); + ignore_value(virNetDevBridgeRemovePort(brname, tapname)); + virErrorRestore(&err); + return -1; + } } return 0; @@ -574,6 +585,7 @@ virNetDevTapReattachBridge(const char *tapname, const unsigned char *vmuuid, const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, + virTristateBool isolatedPort, unsigned int mtu, unsigned int *actualMTU) { @@ -611,6 +623,7 @@ virNetDevTapReattachBridge(const char *tapname, macaddr, vmuuid, virtPortProfile, virtVlan, + isolatedPort, mtu, actualMTU) < 0) return -1; @@ -660,6 +673,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, size_t tapfdSize, const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, + virTristateBool isolatedPort, virNetDevCoalescePtr coalesce, unsigned int mtu, unsigned int *actualMTU, @@ -697,7 +711,8 @@ int virNetDevTapCreateInBridgePort(const char *brname, goto error; if (virNetDevTapAttachBridge(*ifname, brname, macaddr, vmuuid, - virtPortProfile, virtVlan, mtu, actualMTU) < 0) { + virtPortProfile, virtVlan, + isolatedPort, mtu, actualMTU) < 0) { goto error; } diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index cae8e61861..c6bd9285ba 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -65,6 +65,7 @@ virNetDevTapAttachBridge(const char *tapname, const unsigned char *vmuuid, const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, + virTristateBool isolatedPort, unsigned int mtu, unsigned int *actualMTU) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) @@ -77,6 +78,7 @@ virNetDevTapReattachBridge(const char *tapname, const unsigned char *vmuuid, const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, + virTristateBool isolatedPort, unsigned int mtu, unsigned int *actualMTU) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) @@ -91,6 +93,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, size_t tapfdSize, const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, + virTristateBool isolatedPort, virNetDevCoalescePtr coalesce, unsigned int mtu, unsigned int *actualMTU, diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c index 2a552f9f47..25b97f5e04 100644 --- a/tests/bhyvexml2argvmock.c +++ b/tests/bhyvexml2argvmock.c @@ -28,6 +28,7 @@ int virNetDevTapCreateInBridgePort(const char *brname G_GNUC_UNUSED, size_t tapfdSize G_GNUC_UNUSED, const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED, const virNetDevVlan *virtVlan G_GNUC_UNUSED, + virTristateBool isolatedPort G_GNUC_UNUSED, virNetDevCoalescePtr coalesce G_GNUC_UNUSED, unsigned int mtu G_GNUC_UNUSED, unsigned int *actualMTU G_GNUC_UNUSED, -- 2.24.1

On 2/18/20 12:46 PM, Ján Tomko wrote: Whoops! Cut/paste error. (At least I removed the part about being a Navy Seal and having a certain set of skills)

On Sun, Feb 16, 2020 at 11:22:58PM -0500, Laine Stump wrote: consider: if (type != VIR_DOMAIN_NET_TYPE_NETWORK && type != VIR_DOMAIN_NET_TYPE_BRIDGE) Please don't put XML snippets in the error message. How about: ... - isolated ports are not supported ... Same here. Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano