On Tue, 2010-03-09 at 17:30 +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 09, 2010 at 08:19:12AM -0700, Alex Williamson wrote:
> > On Tue, 2010-03-09 at 15:15 +0200, Michael S. Tsirkin wrote:
> > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > > index 5c0093e..01b45ed 100644
> > > --- a/hw/virtio-net.c
> > > +++ b/hw/virtio-net.c
> > > @@ -47,6 +47,7 @@ typedef struct VirtIONet
> > >      uint8_t nomulti;
> > >      uint8_t nouni;
> > >      uint8_t nobcast;
> > > +    uint32_t filtering;
> > >      struct {
> > >          int in_use;
> > >          int first_multi;
> > > @@ -475,12 +476,17 @@ static int receive_filter(VirtIONet *n, const 
> > > uint8_t *buf, int size)
> > >          ptr += sizeof(struct virtio_net_hdr);
> > >      }
> > >  
> > > -    if (!memcmp(&ptr[12], vlan, sizeof(vlan))) {
> > > +    if ((n->filtering & (0x1 << NICCONF_F_VLAN_FILTERING)) &&
> > > +        !memcmp(&ptr[12], vlan, sizeof(vlan))) {
> > >          int vid = be16_to_cpup((uint16_t *)(ptr + 14)) & 0xfff;
> > >          if (!(n->vlans[vid >> 5] & (1U << (vid & 0x1f))))
> > >              return 0;
> > >      }
> > >  
> > > +    if (!(n->filtering & (0x1 << NICCONF_F_MAC_FILTERING))) {
> > > +            return 1;
> > > +    }
> > > +
> > 
> > A filtering flags bitmap is a logical choice here, but I found the
> > overhead to be non-trivial, which is why we have separate variables for
> > the other filtering options.
> 
> You suggest more flags for multicast etc?

I'm suggesting we may get slightly better performance if we use separate
filter_mac and filter_vlan variable flags instead of a single
"filtering" flags bitmap.  However, a couple other ideas... Should we
call receive_filter() as a function pointer so we can make filter
specific versions or remove it completely?  Some overhead to calling it
as a pointer, but could still be a win.  Alternatively we could make
"effective" filter flags which are used by receive_filter(), but
maintained separately from the guest requested flags.  For instance:

virtio_net_handle_rx_mode(...)
{
...
n->alluni_effective = (n->filtering & (0x1 << NICCONF_F_MAC_FILTERING)) ? 
n->alluni : 1;
n->allmulti_effective = (n->filtering & (0x1 << NICCONF_F_MAC_FILTERING)) ? 
n->allmulti : 1;

These could be recreated on loadvm, so we still wouldn't need to save
them.  Question would be whether that creates a sufficiently fast path
through receive_filter().  We'd still need a new flag for vlan filtering
or maybe make the vlan header match bytes settable.

Alex



Reply via email to