On Mon, 2010-05-24 at 14:09 -0600, Eric Blake wrote: > On 05/22/2010 09:31 AM, Stefan Berger wrote: > > This patch adds documentation of the nwfilter subsystem of libvirt to > > the existing (web) docs. I am attaching a PDF in case you don't want to > > read the plain html sources. > > Sometimes, that really is a much easier way to review :) If nothing > else, it proves your patch was well-formed enough to make it through the > conversion to a pdf file without hassle. > > > + > > + <h2><a name="goals">Goals and background</a></h2> > > + > > + <p> > > + The goal of the network filtering XML is to enable administrators > > + of virtualized system to configure and enforce network traffic > > s/of virtualized/of a virtualized/ done. > > > + > > + <h2><a name="nwfconcpts">Concepts</a></h2> > > s/nwfconcpts/nwfconcepts/; does anything else link here yet? > done - nothing links to it, yet > > + <p> > > + The network traffic filtering subsystem enables configuration > > + of network traffic filtering rules on individual network > > + interfaces that are configured for certain types of > > + network configurations. Supported network types are > > + </p> > > + <ul> > > + <li><code>network</code></li> > > + <li><code>ethernet</code> -- must be used in bridging mode</li> > > + <li><code>bridge</code></li> > > + <li><code>direct</code> -- only protocols mac, arp, ip and ipv6 > > + can be filtered</li> > > Unrelated to this patch - does it make sense to add support for a filter > that rejects all traffic that does not contain one of these supported > protocols? > That's really what a rule with priority 1000 would do <rule action='drop' direction='inout' priority='1000'/> Depending on how one has used the chain parameter in the filter node for particularly these prtocols mac, arp etc , the rule would need to be written in separate XML files. > ... > > + <p> > > + Network filters are written in XML and may either contain references > > + to other filters, contain rules for traffic filtering or can > > s/filtering or can/filtering, or/ > ok > > + hold a combination of both. The above referenced filter > > + <code>clean-traffic </code> is a filter that for example only > > s/for example // ok > > + contains references to > > + other filters and no actual filtering rules. Since references to > > + other filters can be used, a <i>tree</i> of filters can be built. > > + > > ... > > + <h3><a name="nwfconcptsvars">Usage of variables in filters</a></h3> > > s/nwfconcptsvars/nwfconceptvars/; does anything else link here yet? > done -- no links, yet > > + > > + <h3><a name="nwfelemsRefs">References to other filers</a></h3> > > s/filers/filters/ Done > > ... > > + <h2><a name="nwfcli">Command line tools</a></h2> > > + <p> > > + The libvirt command line tool <code>virsh</code> has been extended > > + with life-cycle support for network filters. All commands related > > + to the network filtering subsystem start with the prefix > > + <code>nwfilter</code>. The following commands are available: > > + <p> > > + <ul> > > + <li>nwfilter-list : list UUIDs and names of all network filters</li> > > + <li>nwfilter-define : define a new network filter or update an existing one</li> > > + <li>nwfilter-undefine : delete a network filter given its name; it must not be currently in use</li> > > + <li>nwfilter-dumpxml : display a network filter given its name</li> > > + <li>nwfilter-edit : edit a network filter given its name</li> > > + </ul> > > + > > + <h2><a name="nwfexamples">Example network filters</a></h2> > > + <p> > > + The following is a list of example network filters that are > > + automatically installed with libvirt. </p> > > Unusual whitespace. Removed. > > > + <table class="top_table"> > > + <tr> > > + <th> Name </th> > > + <th> Description </th> > > + </tr> > > + <tr> > > + <td> no-arp-spoofing </td> > > + <td> Prevent a VM from spoofing ARP traffic; this filter > > + only allows ARP request and reply messages and enforces > > + that those packets contain the MAC and IP addresses > > + of the VM.</td> > > The section heading of "example network filters" is a bit misleading - I > was expecting to see the actual filter bodies here to form the example. > Either we should document the filter bodies, or we should retitle the > section to "pre-existing network filters". I could go either way, but > obviously re-titling the section is the only one of the two options that > is small enough to not affect my ACK (if you want to go with including > the filter bodies, submit it as a second patch). > I renamed it. :-) > ... > > + The role of the <code>chain</code> attribute in the network filter > > + XML is that internally a new user-defined ebtables table is created > > + that then for example receives all <code>arp</code> traffic coming > > + from or going to a virtual machine, if the chain <code>arp</code> > > + has been specified. Further, a rule is generated in an interface's > > + <code>root</code> chain that directs all ipv4 traffic into the > > + user-defined chain. Therefore, all ARP traffic rules should then be > > + placed into filters specifying this chain. This type of branching > > + into user-define tables is only supported with filtering on the ebtables > > s/define/defined/ fixed > > + The result of these considerations is the following network filter XML: > > + </p> > > +<pre> > > +<filter name='test-eth0'> > > + <!-- reference the clean traffic filter preventing > > s/preventing/to prevent/ what meant in the sense of 'that is preventing', but I fixed it > > > + MAC, IP and ARP spoofing. By not providing > > + and IP address parameter libvirt will detect the > > s/parameter/parameter,/ ok. > > > + IP address the VM is using. --> > > + <filterref filter='clean-traffic'/> > > + > > + <!-- enable TCP ports 22 (ssh) and 80 (http) to be reachable --> > > + <rule action='accept' direction='in'> > > + <tcp dstportstart='22'/> > > + </rule> > > Didn't see mention any earlier that an omitted dstportend defaults to > dstportstart, but it wasn't too hard to figure out from this example. > > > + > > + <h3><a name="nwflimitsmigr">VM Migration</a></h3> > > + <p> > > + VM migration is only supported if the whole filter tree > > + that is referenced by a virtual machine's top level filter > > + is also available on the target host. The network filter > > + <i>clean-traffic</i> > > + for example should be available on all libvirt installations > > + of version 0.8.1 or later and thus enable migration of VMs that > > + for example reference this filter. All other > > + custom filters must be migrated using higher layer software. It is > > + outside the scope of libvirt to ensure that referenced filters > > + on the source system are equivalent to those on the target system > > + and vice versa. > > + <br><br> > > + Migration must occurr between libvirt insallations of version > > s/occurr/occur/; s/insallations/installations/ > fixed > > + 0.8.1 or later in order not to loose the network traffic filters > > s/loose/lose/ fixed > > > + associated with an interface. > > + </p> > > + > > + </body> > > +</html> > > > > Big patch, but much appreciated. You have my ACK, once the nits are > addressed; if we come up with follow-up patches, it will be easier to > see them as incremental diffs than to wait for a v2. > I'll wait until tomorrow morning and will then push it unless I hear other comments. Thanks. Stefan -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list