[libvirt] [PATCH] nwfilter: fix IP address learning
Daniel P. Berrangé
berrange at redhat.com
Fri May 18 17:29:33 UTC 2018
On Fri, May 18, 2018 at 12:59:01PM +0100, Daniel P. Berrangé wrote:
> In a previous commit:
>
> commit d4bf8f415074759baf051644559e04fe78888f8b
> Author: Daniel P. Berrangé <berrange at redhat.com>
> Date: Wed Feb 14 09:43:59 2018 +0000
>
> nwfilter: handle missing switch enum cases
>
> Ensure all enum cases are listed in switch statements, or cast away
> enum type in places where we don't wish to cover all cases.
>
> Reviewed-by: John Ferlan <jferlan at redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>
> we changed a switch in the nwfilter learning thread so that it had
> explict cases for all enum entries. Unfortunately the parameters in the
> method had been declared with incorrect type. The "howDetect" parameter
> does *not* accept "enum howDetect" values, rather it accepts a bitmask
> of "enum howDetect" values, so it should have been an "int" type.
>
> The caller always passes DETECT_STATIC|DETECT_DHCP, so essentially the
> IP addressing learning was completely broken by the above change, as it
> never matched any switch case, hitting the default leading to EINVAL.
Sigh, after applying this fix I find on Fedora 28 at least, nwfilter
will always deadlock on vm shutdown
https://www.redhat.com/archives/libvir-list/2018-May/msg01429.html
I'm guessing we didn't notice this before because Fedora had disabled
TPACKET_V3 in libpcap nutil F28 :-(
So I'm torn about whether to push this or not - broken learning
code for everyone, vs deadlock for anyone with TPACKET_V3
enabled in libpcap (any release since about 2015 seems affected).
>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
> src/nwfilter/nwfilter_learnipaddr.c | 13 ++++++-------
> src/nwfilter/nwfilter_learnipaddr.h | 2 +-
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
> index cc3bfd971c..061b39d72b 100644
> --- a/src/nwfilter/nwfilter_learnipaddr.c
> +++ b/src/nwfilter/nwfilter_learnipaddr.c
> @@ -144,7 +144,7 @@ struct _virNWFilterIPAddrLearnReq {
> char *filtername;
> virHashTablePtr filterparams;
> virNWFilterDriverStatePtr driver;
> - enum howDetect howDetect;
> + int howDetect; /* bitmask of enum howDetect */
>
> int status;
> volatile bool terminate;
> @@ -442,23 +442,22 @@ learnIPAddressThread(void *arg)
> if (techdriver->applyDHCPOnlyRules(req->ifname,
> &req->macaddr,
> NULL, false) < 0) {
> + VIR_DEBUG("Unable to apply DHCP only rules");
> req->status = EINVAL;
> goto done;
> }
> virBufferAddLit(&buf, "src port 67 and dst port 68");
> break;
> - case DETECT_STATIC:
> + default:
> if (techdriver->applyBasicRules(req->ifname,
> &req->macaddr) < 0) {
> + VIR_DEBUG("Unable to apply basic rules");
> req->status = EINVAL;
> goto done;
> }
> virBufferAsprintf(&buf, "ether host %s or ether dst ff:ff:ff:ff:ff:ff",
> macaddr);
> break;
> - default:
> - req->status = EINVAL;
> - goto done;
> }
>
> if (virBufferError(&buf)) {
> @@ -693,7 +692,7 @@ learnIPAddressThread(void *arg)
> * once its IP address has been detected
> * @driver : the network filter driver
> * @howDetect : the method on how the thread is supposed to detect the
> - * IP address; must choose any of the available flags
> + * IP address; bitmask of "enum howDetect" flags.
> *
> * Instruct to learn the IP address being used on a given interface (ifname).
> * Unless there already is a thread attempting to learn the IP address
> @@ -711,7 +710,7 @@ virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
> const char *filtername,
> virHashTablePtr filterparams,
> virNWFilterDriverStatePtr driver,
> - enum howDetect howDetect)
> + int howDetect)
> {
> int rc;
> virThread thread;
> diff --git a/src/nwfilter/nwfilter_learnipaddr.h b/src/nwfilter/nwfilter_learnipaddr.h
> index 06fea5bff8..753aabc594 100644
> --- a/src/nwfilter/nwfilter_learnipaddr.h
> +++ b/src/nwfilter/nwfilter_learnipaddr.h
> @@ -43,7 +43,7 @@ int virNWFilterLearnIPAddress(virNWFilterTechDriverPtr techdriver,
> const char *filtername,
> virHashTablePtr filterparams,
> virNWFilterDriverStatePtr driver,
> - enum howDetect howDetect);
> + int howDetect);
>
> bool virNWFilterHasLearnReq(int ifindex);
> int virNWFilterTerminateLearnReq(const char *ifname);
> --
> 2.17.0
>
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 :|
More information about the libvir-list
mailing list