[PATCH] nwfilter: Remove 'qemu-announce-self' example

Peter Krempa via Devel posted 1 patch 5 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/b9c73f8c73c6310d3ffbf077da5183c7d9d4253a.1751871786.git.pkrempa@redhat.com
There is a newer version of this series
docs/firewall.rst                            |  1 -
docs/formatnwfilter.rst                      |  2 +-
src/nwfilter/xml/clean-traffic-gateway.xml   |  2 +-
src/nwfilter/xml/clean-traffic.xml           |  2 +-
src/nwfilter/xml/meson.build                 |  1 -
src/nwfilter/xml/qemu-announce-self-rarp.xml |  2 ++
src/nwfilter/xml/qemu-announce-self.xml      | 13 -------------
7 files changed, 5 insertions(+), 18 deletions(-)
delete mode 100644 src/nwfilter/xml/qemu-announce-self.xml
[PATCH] nwfilter: Remove 'qemu-announce-self' example
Posted by Peter Krempa via Devel 5 months, 2 weeks ago
From: Peter Krempa <pkrempa@redhat.com>

The example allows packets sent by qemu after migration with broken
protocol ID. The proper self announce is handled via
'qemu-announce-self-rarp'.

The qemu bug was addressed by f8778a7785d530515b0db39 (released as
v0.13.0). As we no longer support such old qemus, and allowing broken
packets makes no sense remove the filter, and adjust the existing ones
to refer to the proper name.

Closes: https://gitlab.com/libvirt/libvirt/-/issues/792
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 docs/firewall.rst                            |  1 -
 docs/formatnwfilter.rst                      |  2 +-
 src/nwfilter/xml/clean-traffic-gateway.xml   |  2 +-
 src/nwfilter/xml/clean-traffic.xml           |  2 +-
 src/nwfilter/xml/meson.build                 |  1 -
 src/nwfilter/xml/qemu-announce-self-rarp.xml |  2 ++
 src/nwfilter/xml/qemu-announce-self.xml      | 13 -------------
 7 files changed, 5 insertions(+), 18 deletions(-)
 delete mode 100644 src/nwfilter/xml/qemu-announce-self.xml

diff --git a/docs/firewall.rst b/docs/firewall.rst
index 26474d3317..81114d2c95 100644
--- a/docs/firewall.rst
+++ b/docs/firewall.rst
@@ -285,7 +285,6 @@ useful rules:
    fb57c546-76dc-a372-513f-e8179011b48a  no-mac-spoofing
    dba10ea7-446d-76de-346f-335bd99c1d05  no-other-l2-traffic
    f5c78134-9da4-0c60-a9f0-fb37bc21ac1f  no-other-rarp-traffic
-   7637e405-4ccf-42ac-5b41-14f8d03d8cf3  qemu-announce-self
    9aed52e7-f0f3-343e-fe5c-7dcb27b594e5  qemu-announce-self-rarp

 Most of these are just building blocks. The interesting one here is
diff --git a/docs/formatnwfilter.rst b/docs/formatnwfilter.rst
index 13e9a791af..e50497aaf8 100644
--- a/docs/formatnwfilter.rst
+++ b/docs/formatnwfilter.rst
@@ -438,7 +438,7 @@ several other filters.
      <filterref filter='allow-incoming-ipv4'/>
      <filterref filter='no-arp-spoofing'/>
      <filterref filter='no-other-l2-traffic'/>
-     <filterref filter='qemu-announce-self'/>
+     <filterref filter='qemu-announce-self-rarp'/>
    </filter>

 To reference another filter, the XML node ``filterref`` needs to be provided
diff --git a/src/nwfilter/xml/clean-traffic-gateway.xml b/src/nwfilter/xml/clean-traffic-gateway.xml
index b8c204041a..1768a67697 100644
--- a/src/nwfilter/xml/clean-traffic-gateway.xml
+++ b/src/nwfilter/xml/clean-traffic-gateway.xml
@@ -30,5 +30,5 @@
     <filterref filter='no-other-l2-traffic'/>

     <!-- allow qemu to send a self-announce upon migration end -->
-    <filterref filter='qemu-announce-self'/>
+    <filterref filter='qemu-announce-self-rarp'/>
 </filter>
diff --git a/src/nwfilter/xml/clean-traffic.xml b/src/nwfilter/xml/clean-traffic.xml
index b8cde9c560..b0530da70a 100644
--- a/src/nwfilter/xml/clean-traffic.xml
+++ b/src/nwfilter/xml/clean-traffic.xml
@@ -25,6 +25,6 @@
    <filterref filter='no-other-l2-traffic'/>

    <!-- allow qemu to send a self-announce upon migration end -->
-   <filterref filter='qemu-announce-self'/>
+   <filterref filter='qemu-announce-self-rarp'/>

 </filter>
diff --git a/src/nwfilter/xml/meson.build b/src/nwfilter/xml/meson.build
index 0d96c54ebe..de3f205a7c 100644
--- a/src/nwfilter/xml/meson.build
+++ b/src/nwfilter/xml/meson.build
@@ -22,7 +22,6 @@ nwfilter_xml_files = [
   'no-other-l2-traffic.xml',
   'no-other-rarp-traffic.xml',
   'qemu-announce-self-rarp.xml',
-  'qemu-announce-self.xml',
 ]

 install_data(nwfilter_xml_files, install_dir: sysconfdir / 'libvirt' / 'nwfilter')
diff --git a/src/nwfilter/xml/qemu-announce-self-rarp.xml b/src/nwfilter/xml/qemu-announce-self-rarp.xml
index b7a848ad0f..db7b650320 100644
--- a/src/nwfilter/xml/qemu-announce-self-rarp.xml
+++ b/src/nwfilter/xml/qemu-announce-self-rarp.xml
@@ -11,4 +11,6 @@
           arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC'
           arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/>
   </rule>
+
+  <filterref filter='no-other-rarp-traffic'/>
 </filter>
diff --git a/src/nwfilter/xml/qemu-announce-self.xml b/src/nwfilter/xml/qemu-announce-self.xml
deleted file mode 100644
index 352db500de..0000000000
--- a/src/nwfilter/xml/qemu-announce-self.xml
+++ /dev/null
@@ -1,13 +0,0 @@
-<filter name='qemu-announce-self' chain='root'>
-    <!-- as of 4/26/2010 qemu sends out a bogus packet with
-         wrong rarp protocol ID -->
-    <!-- accept what is being sent now -->
-    <rule action='accept' direction='out'>
-        <mac protocolid='0x835'/>
-    </rule>
-
-    <!-- accept if it was changed to rarp -->
-    <filterref filter='qemu-announce-self-rarp'/>
-    <filterref filter='no-other-rarp-traffic'/>
-
-</filter>
-- 
2.49.0
Re: [PATCH] nwfilter: Remove 'qemu-announce-self' example
Posted by Daniel P. Berrangé via Devel 5 months, 2 weeks ago
On Mon, Jul 07, 2025 at 09:03:06AM +0200, Peter Krempa via Devel wrote:
> From: Peter Krempa <pkrempa@redhat.com>
> 
> The example allows packets sent by qemu after migration with broken
> protocol ID. The proper self announce is handled via
> 'qemu-announce-self-rarp'.
> 
> The qemu bug was addressed by f8778a7785d530515b0db39 (released as
> v0.13.0). As we no longer support such old qemus, and allowing broken
> packets makes no sense remove the filter, and adjust the existing ones
> to refer to the proper name.
> 
> Closes: https://gitlab.com/libvirt/libvirt/-/issues/792
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>


> diff --git a/src/nwfilter/xml/qemu-announce-self.xml b/src/nwfilter/xml/qemu-announce-self.xml
> deleted file mode 100644
> index 352db500de..0000000000
> --- a/src/nwfilter/xml/qemu-announce-self.xml
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -<filter name='qemu-announce-self' chain='root'>
> -    <!-- as of 4/26/2010 qemu sends out a bogus packet with
> -         wrong rarp protocol ID -->
> -    <!-- accept what is being sent now -->
> -    <rule action='accept' direction='out'>
> -        <mac protocolid='0x835'/>
> -    </rule>
> -
> -    <!-- accept if it was changed to rarp -->
> -    <filterref filter='qemu-announce-self-rarp'/>
> -    <filterref filter='no-other-rarp-traffic'/>
> -
> -</filter>

Deleting this filter entirely risks upgrade problems for apps if
they're referencing, which is quite possible given it was one of
our standard filters for decades.

Perhaps keep it, but cut down rules such that it acts exactly as
an alias of qemu-announce-self-rarp.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] nwfilter: Remove 'qemu-announce-self' example
Posted by Peter Krempa via Devel 5 months, 2 weeks ago
On Mon, Jul 07, 2025 at 09:19:56 +0100, Daniel P. Berrangé wrote:
> On Mon, Jul 07, 2025 at 09:03:06AM +0200, Peter Krempa via Devel wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> > 
> > The example allows packets sent by qemu after migration with broken
> > protocol ID. The proper self announce is handled via
> > 'qemu-announce-self-rarp'.
> > 
> > The qemu bug was addressed by f8778a7785d530515b0db39 (released as
> > v0.13.0). As we no longer support such old qemus, and allowing broken
> > packets makes no sense remove the filter, and adjust the existing ones
> > to refer to the proper name.
> > 
> > Closes: https://gitlab.com/libvirt/libvirt/-/issues/792
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> 
> 
> > diff --git a/src/nwfilter/xml/qemu-announce-self.xml b/src/nwfilter/xml/qemu-announce-self.xml
> > deleted file mode 100644
> > index 352db500de..0000000000
> > --- a/src/nwfilter/xml/qemu-announce-self.xml
> > +++ /dev/null
> > @@ -1,13 +0,0 @@
> > -<filter name='qemu-announce-self' chain='root'>
> > -    <!-- as of 4/26/2010 qemu sends out a bogus packet with
> > -         wrong rarp protocol ID -->
> > -    <!-- accept what is being sent now -->
> > -    <rule action='accept' direction='out'>
> > -        <mac protocolid='0x835'/>
> > -    </rule>
> > -
> > -    <!-- accept if it was changed to rarp -->
> > -    <filterref filter='qemu-announce-self-rarp'/>
> > -    <filterref filter='no-other-rarp-traffic'/>
> > -
> > -</filter>
> 
> Deleting this filter entirely risks upgrade problems for apps if
> they're referencing, which is quite possible given it was one of
> our standard filters for decades.

Hmm, yeah, while I did think about upgrades themselves where this should
work as we *copy* the files from 'datadir' to 'sysconfdir' in %post,
thus the old file will be left around for cases where this was
installed, if someone actually used this in their filter definition we'd
break them on new instalation.

> Perhaps keep it, but cut down rules such that it acts exactly as
> an alias of qemu-announce-self-rarp.

Yeah, this should be straightforward if we want to keep user-defined
filter rules depending on our examples working.

I can find arguments for etiher behaviour, but I'm leaning towards
making this just include the '-rarp' version as it was the intended way.
Re: [PATCH] nwfilter: Remove 'qemu-announce-self' example
Posted by Daniel P. Berrangé via Devel 5 months, 2 weeks ago
On Mon, Jul 07, 2025 at 12:16:07PM +0200, Peter Krempa wrote:
> On Mon, Jul 07, 2025 at 09:19:56 +0100, Daniel P. Berrangé wrote:
> > On Mon, Jul 07, 2025 at 09:03:06AM +0200, Peter Krempa via Devel wrote:
> > > From: Peter Krempa <pkrempa@redhat.com>
> > > 
> > > The example allows packets sent by qemu after migration with broken
> > > protocol ID. The proper self announce is handled via
> > > 'qemu-announce-self-rarp'.
> > > 
> > > The qemu bug was addressed by f8778a7785d530515b0db39 (released as
> > > v0.13.0). As we no longer support such old qemus, and allowing broken
> > > packets makes no sense remove the filter, and adjust the existing ones
> > > to refer to the proper name.
> > > 
> > > Closes: https://gitlab.com/libvirt/libvirt/-/issues/792
> > > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > 
> > 
> > > diff --git a/src/nwfilter/xml/qemu-announce-self.xml b/src/nwfilter/xml/qemu-announce-self.xml
> > > deleted file mode 100644
> > > index 352db500de..0000000000
> > > --- a/src/nwfilter/xml/qemu-announce-self.xml
> > > +++ /dev/null
> > > @@ -1,13 +0,0 @@
> > > -<filter name='qemu-announce-self' chain='root'>
> > > -    <!-- as of 4/26/2010 qemu sends out a bogus packet with
> > > -         wrong rarp protocol ID -->
> > > -    <!-- accept what is being sent now -->
> > > -    <rule action='accept' direction='out'>
> > > -        <mac protocolid='0x835'/>
> > > -    </rule>
> > > -
> > > -    <!-- accept if it was changed to rarp -->
> > > -    <filterref filter='qemu-announce-self-rarp'/>
> > > -    <filterref filter='no-other-rarp-traffic'/>
> > > -
> > > -</filter>
> > 
> > Deleting this filter entirely risks upgrade problems for apps if
> > they're referencing, which is quite possible given it was one of
> > our standard filters for decades.
> 
> Hmm, yeah, while I did think about upgrades themselves where this should
> work as we *copy* the files from 'datadir' to 'sysconfdir' in %post,
> thus the old file will be left around for cases where this was
> installed, if someone actually used this in their filter definition we'd
> break them on new instalation.

In data center / cloud deployments I think "new provisioned host" is a
relatively common thing, and it might be surprising for admins to find
the new hosts are incomplete vs old hosts.

> > Perhaps keep it, but cut down rules such that it acts exactly as
> > an alias of qemu-announce-self-rarp.
> 
> Yeah, this should be straightforward if we want to keep user-defined
> filter rules depending on our examples working.
> 
> I can find arguments for etiher behaviour, but I'm leaning towards
> making this just include the '-rarp' version as it was the intended way.
>

So basically nothing more than:

  <filter name='qemu-announce-self' chain='root'>
    <filterref filter='qemu-announce-self-rarp'/>
  </filter>

...unless some future QEMU needs to announce a packet that is not rarp
based

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] nwfilter: Remove 'qemu-announce-self' example
Posted by Pavel Hrdina via Devel 5 months, 2 weeks ago
On Mon, Jul 07, 2025 at 09:03:06AM +0200, Peter Krempa via Devel wrote:
> From: Peter Krempa <pkrempa@redhat.com>
> 
> The example allows packets sent by qemu after migration with broken
> protocol ID. The proper self announce is handled via
> 'qemu-announce-self-rarp'.
> 
> The qemu bug was addressed by f8778a7785d530515b0db39 (released as
> v0.13.0). As we no longer support such old qemus, and allowing broken
> packets makes no sense remove the filter, and adjust the existing ones
> to refer to the proper name.
> 
> Closes: https://gitlab.com/libvirt/libvirt/-/issues/792
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  docs/firewall.rst                            |  1 -
>  docs/formatnwfilter.rst                      |  2 +-
>  src/nwfilter/xml/clean-traffic-gateway.xml   |  2 +-
>  src/nwfilter/xml/clean-traffic.xml           |  2 +-
>  src/nwfilter/xml/meson.build                 |  1 -
>  src/nwfilter/xml/qemu-announce-self-rarp.xml |  2 ++
>  src/nwfilter/xml/qemu-announce-self.xml      | 13 -------------
>  7 files changed, 5 insertions(+), 18 deletions(-)
>  delete mode 100644 src/nwfilter/xml/qemu-announce-self.xml

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>