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@xxxxxxxxxx> > 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@xxxxxxxxxx> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > 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@xxxxxxxxxx> > --- > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
