[ovs-dev] [PATCH 7/7] datapath: Report kernel's flow key when passing packets up to userspace.

Ben Pfaff blp at nicira.com
Tue Dec 28 23:13:13 UTC 2010


On Tue, Dec 28, 2010 at 06:01:31PM -0500, Jesse Gross wrote:
> On Tue, Dec 28, 2010 at 4:47 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Tue, Dec 28, 2010 at 03:29:09PM -0500, Jesse Gross wrote:
> >> On Thu, Dec 23, 2010 at 8:15 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> > +       atomic_inc(&p->sflow_pool);
> >>
> >> Somewhat unrelated but: why do we need to separately track an sflow
> >> pool?  Isn't it simply the number of packets received on this port?
> >> Also, since it is only 32-bits, it's at risk of overflow.
> >
> > It's the number of packets that were received on the port, that had a
> > matching flow, that were not dropped by loop detection, since the time
> > that sflow sampling was enabled on the port.
> >
> 
> The matching flow part doesn't seem quite right because it will miss
> packets that are handled by userspace.  I don't think an external
> collector should care where the packets are handled.  Userspace should
> know when sflow was enabled on a port, so it should be able to
> compensate manually.  Hopefully, loop detection should be a pretty
> rare case...
> 
> It seems to me that this is something that could be done in userspace.
>  The kernel piece could then become a more generic sample action.
> 
> > We can probably avoid the atomic increment somehow, but this seems like
> > something to do elsewhere.
> 
> Yes, if this stays in the kernel it should become percpu data.
> 
> > I wanted to make sflow_pool a 64-bit value originally, but the InMon
> > folks told me that wraparound didn't matter so many times that I
> > eventually believed them.
> 
> It can be compensated for, if necessary.  Maybe the sflow protocol
> only has a 32-bit field.
> 
> >
> > I also want to get rid of the need to send down the actions to userspace
> > here.  Ugh.
> 
> Yes, that would be nice as well.

I'll make a mental note that this stuff needs attending to.




More information about the dev mailing list