Re: [libvirt] [PATCH V1 1/9] Add support for VLAN filtering
On 11/15/2011 07:46 PM, Eric Blake wrote:
On 10/26/2011 09:11 AM, Stefan Berger wrote:
Apologies for delaying so long on reviewing this.
> This patch adds support for filtering of VLAN (802.1Q) traffic to
the
> parser and makes us of the ebtables support for VLAN filtering. This code
> now enables the filtering of traffic in chains with prefix 'vlan'.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> docs/schemas/nwfilter.rng | 47 +++++++++++++
> src/conf/nwfilter_conf.c | 102 ++++++++++++++++++++++++++++++
> src/conf/nwfilter_conf.h | 17 +++++
> src/nwfilter/nwfilter_ebiptables_driver.c | 35 ++++++++++
> 4 files changed, 201 insertions(+)
>
This didn't apply cleanly for me. Am I missing the review of another
pre-requisite series? Or is it just something where you need to rebase
and post a v2 for easier review?
Yes, sorry for the confusion. This builds on top of the following two
series:
https://www.redhat.com/archives/libvir-list/2011-October/msg01227.html
then came this one (already ACKed):
https://www.redhat.com/archives/libvir-list/2011-October/msg01360.html
>
> +static bool
> +checkVlanVlanID(enum attrDatatype datatype, union data *value,
> + virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED,
ATTRIBUTE_UNUSED not necessary here, since...
> + nwItemDesc *item ATTRIBUTE_UNUSED)
> +{
> + int32_t res;
> +
> + res = value->ui;
> + if (res< 0 || res> 4095) {
> + res = -1;
> + }
> +
> + if (res != -1) {
> + nwf->p.vlanHdrFilter.dataVlanID.u.u16 = res;
...this uses nwf. Similar comment applies elsewhere in the patch.
Thanks. Fixing it.
Overall, it looks sane, but I didn't compile test it.
Stefan