Re: [libvirt] [libvirt PATCHv6 1/1] add DHCP snooping

2012-03-22 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 03/22/2012 12:22:20 PM: > > I tried it. It doesn't apply more than one IP address. The code also > doesn't apply cleanly to the tip. > >Stefan Stefan, I did a git pull yesterday to which this patch is applied; here is the last entry before the patch

Re: [libvirt] [libvirt PATCHv6 1/1] add DHCP snooping

2012-03-22 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 03/22/2012 03:04:53 PM: > > I have some concerns about the cancelation of the thread. It can > hold the snoop lock and get cancelled while holding it. Next time > that lock is grabbed we will get a deadlock... > The snoop lock is acquired in virNWFilterDHCPSno

Re: [libvirt] [libvirt PATCHv6 1/1] add DHCP snooping

2012-03-22 Thread David Stevens
Eric Blake wrote on 03/22/2012 03:54:31 PM: > > pthread_cancel() tends to imply that you are properly managing signal > blocking across threads; we haven't used it anywhere else in libvirt, > and I'm extremely wary of pulling it in now, as there's probably a lot > of subtle bugs that it would ex

Re: [libvirt] [libvirt PATCHv6 1/1] add DHCP snooping

2012-03-22 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 03/22/2012 05:00:45 PM: > Maybe we should go with the previous code from a while ago which was > setting a flag for the thread to die. It caused other work-arounds > to become necessary but at least we don't have to deal with possibly > async. deaths of threads h

Re: [libvirt] [libvirt PATCHv6 1/1] add DHCP snooping

2012-03-22 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 03/22/2012 05:33:41 PM: > > Ok. > An idea may be that the threat has to 'find' its snoop request in a > global list every time it processes a packet. Once it cannot find it > anymore, it dies. Removing the request from the global list would be > the way to termi

Re: [libvirt] [RFC PATCHv2 4/9] make default chain policy "DROP"

2011-10-05 Thread David Stevens
"Daniel P. Berrange" wrote on 10/05/2011 08:43:45 AM: > This sounds like it is introducing a backwards compatibility problem > wrt older libvirt deployments using NW Filters. I don't think so. Again, only if someone has modified a standard filter would they have to make those same modification

Re: [libvirt] [RFC PATCHv2 3/9] reverse sense of address matching

2011-10-05 Thread David Stevens
"Daniel P. Berrange" wrote on 10/05/2011 08:41:56 AM: > This looks like it is breaking compatibility of NWFilter XML with > previously deployed libvirt releases. I think only if someone has modified the standard filters. Then they would have to apply those modifications to the new set.

Re: [libvirt] [RFC PATCHv2 4/9] make default chain policy "DROP"

2011-10-05 Thread David Stevens
"Daniel P. Berrange" wrote on 10/05/2011 09:15:08 AM: > What if they have created their own custom filters and written their > filter on the assumption that the default policy was ACCEPT ? Surely > this change will break their filter ? If their filter has "ACCEPT" or "DROP", that will

Re: [libvirt] [libvirt PATCHv3 00/10] DHCP snooping support for libvirt

2011-10-12 Thread David Stevens
Stefan Berger wrote on 10/12/2011 02:02:59 PM: >The problem we're having at the moment is that it's not possible to > evaluate fields of packets that may have more than one possible value. > This is the general problem, the specific one being allowing multiple > MAC or IP addresses. Stef

Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy "DROP"

2011-10-17 Thread David Stevens
Stefan Berger wrote on 10/17/2011 08:50:14 AM: > I agree with Daniel's previous comments that this could introduce > compatibility problems. It would be best not to change it or if really > need be later on introduce an XML attribute for a chain that allows to > choose whether the default po

Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification

2011-10-17 Thread David Stevens
Stefan Berger wrote on 10/17/2011 09:07:12 AM: > On 10/12/2011 03:50 PM, David L Stevens wrote: > >This patch adds the internal capability to add rules to existing > > chains instead of using temporary chains and to generate placeholders for > > chains that are referenced without generating

Re: [libvirt] [libvirt PATCHv3 07/10] support variable value changing

2011-10-17 Thread David Stevens
Stefan Berger wrote on 10/17/2011 09:17:21 AM: > > +int > > +virNWFilterChangeVar(virConnectPtr conn, > > +if (virNWFilterHashTablePut(vars, var, value, 1)) { > > +virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("Cound not add " > > + "variable \"

Re: [libvirt] [libvirt PATCHv3 05/10] allow chain modification

2011-10-17 Thread David Stevens
Stefan Berger wrote on 10/17/2011 10:31:29 AM: > > was not. > Yes, then I understood this correctly. See the other mails regarding the > problems I am seeing with it. If there was a way to figure out at what > position to insert a rule into an existing chain, i.e. at position 5, > rather th

Re: [libvirt] [libvirt PATCHv3 04/10] make default chain policy "DROP"

2011-10-17 Thread David Stevens
Stefan Berger wrote on 10/17/2011 10:29:08 AM: > Yes, '_at_the_end_', that's what I thought. I am not sure whether this > particular requirement is the best way to proceed since obviously you > cannot have any other rules with lesser priority after the ones doing > the 'return' -- whatever th

Re: [libvirt] [PATCH V2 00/10] Make inner workings of nwfilters more flexible + extensions

2011-10-19 Thread David Stevens
Stefan, Can't you achieve the same thing by reserving an early blockof priorities (and a late one, for system stuff that should be done late)? If you use negative numbers, then you lose the capability ofever extending priorities to interpret the negative number as "from the

Re: [libvirt] [PATCH V2 00/10] Make inner workings of nwfilters more flexible + extensions

2011-10-19 Thread David Stevens
-Stefan Berger wrote: -    >  >The problem is that at the moment rules (in the 'root' table) can>have priorities [0, 1000]. So nothing prevents one to write a>rule>with priority 0. However, due to how nwfilters works right now>the>jumps into the protocol-specific tables

Re: [libvirt] [PATCH V2 00/10] Make inner workings of nwfilters more flexible + extensions

2011-10-19 Thread David Stevens
-Matthias Bolte wrote: ->>Well, you miss the point that nwfilters is meant as a general>firewall>interface. ebtables/iptables just happens to be an implementation of>this interface. Using ebtables/iptables specific shell scripts would>replace the generic interface with something specific t

Re: [libvirt] [libvirt PATCH] support continue/return targets in nwfilter

2011-10-20 Thread David Stevens
Eric, Thanks -- looks good to me. "return" is useful for doing multiple chains on one packet -- "drop" if it isn't acceptable and "return" to do further checks in other chains for the acceptable ones. The current fixed-set of protocol chains are mutually exclusive which

Re: [libvirt] [libvirt PATCHv4 0/2] DHCP Snooping support for libvirt

2011-10-25 Thread David Stevens
Stefan, Thanks for reviewing. Stefan Berger wrote on 10/25/2011 04:59:16 AM: > I hope that this code will be able to support multiple IP addresses once > my patches have gone in. I labelled in the code with comments the places that need to be revised (3, I believe) for multip

Re: [libvirt] [libvirt PATCHv3 03/10] reverse sense of address matching

2011-10-25 Thread David Stevens
Stefan Berger wrote on 10/25/2011 12:01:05 PM: > So the intention here was to remove the 'arp' chain. With it staying now > I suppose this patch and the allow-arpmac and allow-arpip aren't needed. > > diff --git a/examples/xml/nwfilter/allow-arpip.xml b/examples/xml/ > nwfilter/allow-arpip.xml

Re: [libvirt] [libvirt PATCHv4 1/2] add DHCP snooping

2011-10-26 Thread David Stevens
Stefan Berger wrote on 10/26/2011 11:32:25 AM: > >> diff --git a/examples/xml/nwfilter/no-ip-spoofing.xml > >> b/examples/xml/nwfilter/no-ip-spoofing.xml > >> index b8c94c8..e5969a0 100644 > >> --- a/examples/xml/nwfilter/no-ip-spoofing.xml > >> +++ b/examples/xml/nwfilter/no-ip-spoofing.xml >

Re: [libvirt] [libvirt PATCHv4 1/2] add DHCP snooping

2011-10-27 Thread David Stevens
Stefan Berger wrote on 10/25/2011 05:50:09 AM: > The below algorithm also seems to assume that the VM does not send > 802.1Q (VLAN) headers. I remember having had problems when trying to > receive 802.1Q headers when the VM's interface is on a bridge and the > remote traffic comes through the

Re: [libvirt] [PATCH 0/9] add DHCP snooping support to nwfilter

2011-05-09 Thread David Stevens
Eric Blake wrote on 05/09/2011 01:41:37 PM: > Can you configure your mailer to send related patches threaded to one > another (or at least all as a reply to the 0/9 cover-letter), rather > than starting an independent thread for each mail in the series? 'git > send-email' can do this. Also, som

Re: [libvirt] [PATCH 9/9] add DHCP snooping support to nwfilter

2011-05-10 Thread David Stevens
"Daniel P. Berrange" wrote on 05/10/2011 02:28:25 AM: > From: "Daniel P. Berrange" > To: David Stevens/Beaverton/IBM@IBMUS > Cc: libvirt-l...@redhat.com > Date: 05/10/2011 02:32 AM > Subject: Re: [libvirt] [PATCH 9/9] add DHCP snooping support to nwfilter

Re: [libvirt] [PATCH 0/9] add DHCP snooping support to nwfilter

2011-05-11 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 05/11/2011 11:59:21 AM: > Looking at patch 8 I would assume you need to store the IP leases > you track into > a file so you can handle the cases of libvirt restart while a VM is > running. How > does the DHCP snooping currently deal with libvirt restarts or a

Re: [libvirt] [PATCH 3/9] add DHCP snooping support to nwfilter

2011-05-11 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 05/11/2011 12:20:49 PM: > > The mac chain is there for supporting multiple MAC addresses per > interface. What is the use case for having > multiple MAC address on an interface and how do I set this up in a > Linux guest for example? I don't know if Linu

Re: [libvirt] [PATCH 4/9] add DHCP snooping support to nwfilter

2011-05-11 Thread David Stevens
Stefan Berger/Watson/IBM wrote on 05/11/2011 12:32:41 PM: > > So now this command puts the default policy of every ebtables chain > to end with an implicit drop. What if I had previously > created a filter assuming an implicit accept, which is the current > behavior? Now that filter wouldn't work

Re: [libvirt] [PATCH 9/9] add DHCP snooping support to nwfilter

2011-05-18 Thread David Stevens
Daniel Veillard wrote on 05/17/2011 08:47:11 PM: > Like Dan I'm worried by removing this functionality. As far as I > know most switches learn IP from their clients using ARP snooping, > this is I think more resilient and minimize disruption in case of > port switching. Daniel, Althou

Re: [libvirt] [PATCH 2/9] add DHCP snooping support to nwfilter

2011-05-23 Thread David Stevens
Stefan Berger wrote on 05/23/2011 01:09:51 PM: > For the other ARP requests I am not sure whether the VM needs to see all > of them. If a VM sees an ARP request on an interface not directed for > any of its IP addresses, why deliver the request at all? The VM cannot > respond to it. Since w