OK, that sounds reasonable to me. Can you update the comments to reflect that sFlow should be OK now - assuming that you think it is. I see two: ofproto/ofproto-dpif.c:port_construct() and the one in ofproto/tunnel.c about metadata for packet outs, which was intended to be for sFlow.
On Thu, Apr 25, 2013 at 8:18 PM, Neil McKee <neil.mc...@inmon.com> wrote: > Thank you for spending time thinking about this. > > There is no loss of information in changing from a per-interface sampling > model to a per-bridge sampling model. > > If there is a difference it's only on the configuration side, and since the > OVS sFlow config is already per-bridge, there is no conflict there. > > Hypothetically, if the datapath offered separate per-physical-port sampling > probability and we always knew what physical port a packet came in/went out > on, then we would have the choice of staying with the per-interface model and > changing the config schema to expose it. However that seems like a stretch > -- and not worth the effort at all if you remember that the normal practice > for sFlow monitoring is to configure the same sampling rate on every port of > every switch in the LAN. > > Neil > > > > On Apr 25, 2013, at 5:06 PM, Jesse Gross <je...@nicira.com> wrote: > >> OK, thanks for the explanation. I guess the part that I'm not entirely >> sure about is what is the tradeoff in modeling this as being >> per-bridge rather than per-port? It seems like like in the case of >> per-bridge sampling you are still including the ifIndex with the >> sample metadata so you don't lose any information. Is there any >> disadvantage? >> >> On Wed, Apr 24, 2013 at 2:04 PM, Neil Mckee <neil.mc...@inmon.com> wrote: >>> Actually it wasn't just tunnel ports. Anything that didn't have an ifIndex >>> was mapping to the same sFlow datasource. Even if it was on another bridge. >>> >>> Besides, sFlow wouldn't want to model every tunnel as a separate datasource >>> either -- even if they did have ifIndex numbers. There might be thousands. >>> The idea is to have a small number of fairly-persistent datasources and >>> report on tunnels by annotating individual packet samples (see below). >>> >>> I think the original thinking (years ago) was that we might ignore all >>> samples that did not ingress on an ifindex-interface, but that's not what >>> is happening, and it's not what we want to happen now anyway. Now it's >>> quite common to see ingress-sampled packets that we really want to report >>> on where that ingress port doesn't have an ifIndex (GRE tunnels being just >>> the most obvious example). So if we drop the per-interface-sampler >>> representation and adopt one that more accurately represents what is going >>> on below then it falls out correctly, and still conforms to the sFlow v5 >>> standard. >>> >>> So looking at the code, the hash-table that the ofproto-sflow module >>> maintains to look up odp_port -> struct of_port becomes just a cache of the >>> ifindex ports. It is still needed so that the counter-push data-sources >>> can retrieve the latest stats from the netdev, and so that the packet >>> samples can have their ingress ifindex filled in where applicable. >>> However if an ingress sample comes in for some other port and there is no >>> lookup in the table then it's no longer a show-stopper. It just means that >>> the packet sample goes out with inputPort = 0 (meaning "ifindex unknown "). >>> >>> Ideally, we would want to know the original ifIndex of the physical port >>> where that tunnel first entered the hypervisor, but that seems hard to >>> know. >>> >>> Looking ahead, it would be good to know the details of the tunnel too, so >>> we could annotate the packet-sample with another structure: >>> http://sflow.org/sflow_tunnels.txt >>> >>> but that might be part of a broader discussion where we look at ways to >>> expose more state to the ofproto-sflow module. For example, I am >>> currently looking at how the bundle information might be exposed so that we >>> can also fill in these structures: >>> http://www.sflow.org/sflow_lag.txt >>> >>> And sFlow has standard structures for reporting on MPLS too: >>> http://www.sflow.org/SFLOW-STRUCTS5.txt >>> >>> It seems like there is choice between (1) pushing the information down to >>> ofproto-sflow on a state-change (as we do with that odp_port -> of_port >>> lookup) or (2) providing callback hooks that allow ofproto-dpif-sflow to >>> ask ofproto-dpif questions about tunnels, bundles and more. Right now it >>> looks to me like the latter would be better, because it allows >>> ofproto-dpif to completely control what is exposed and keep a tight grip on >>> all it's internal pointers. But maybe there are other architectural >>> considerations? >>> >>> Neil >>> >>> >>> On Apr 24, 2013, at 11:09 AM, Jesse Gross wrote: >>> >>>> On Wed, Apr 24, 2013 at 10:46 AM, Neil Mckee <neil.mc...@inmon.com> wrote: >>>>> Adjust sFlow packet samplers to better reflect the underlying per-bridge >>>>> sampling that is really happening, rather than modeling it as >>>>> per-interface >>>>> sampling. This fixes an aliasing issue: tunnel traffic sampled on >>>>> different >>>>> bridges was all being reported against the same sFlow datasource. >>>>> >>>>> The sFlow counter samples remain unchanged, reporting only on interfaces >>>>> that have valid ifIndex numbers. >>>>> >>>>> See definition of SFlowDataSource in http://sflow.org/sflow_version_5.txt. >>>>> >>>>> Signed-off-by: Neil McKee <neil.mc...@inmon.com> >>>> >>>> It actually is possible to disambiguate between tunnel ports in order >>>> to continue reporting on a per-port basis, have you considered doing >>>> that? >>> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
