[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