On Fri, Nov 18, 2016 at 12:35:13PM -0700, Neil McKee wrote: > That sounds like a robust way to do it, yes. Does it have the desired > effect? Does it eliminate this structure for all the virtual ethernet > ports? I believe there should only be <= 1 physical ethernet port on each
It eliminates this structure for all interfaces for which those counters are not available (its value is equal to UINT_MAX). I tested that for linux tap interface and dpdk vhost-user. Both are virtual, but for the first one these ethernet counters are available, but for the second one they aren't. Basically the logic what statistics are avaialable for the given interface is performed on the netdev (netdev-dpdk, netdev-linux,...) level. > OVS bridge, so the ethernet stats block should only be included for those. > > It looks like ifType=ethernetcsmacd is hard-coded at this point: > https://github.com/openvswitch/ovs/blob/master/ofproto/ofproto-dpif-sflow.c#L317 > But maybe we should at least add a comment to make sure we never fall into > the trap of sending an ethernet structure for something that has a > symbol-errors counter but is not strictly ethernet (e.g. infiniband or > fiber-channel). It's not as simple as testing for ifType==6 because it > could be ifType 7,26,62,69 or 117 and still be ethernet. So I think a > comment would be OK for now. What do you think? > I'm thinking of adding something like this: /* XXX Ethernet counters don't make sense for all interfaces, so let's check if there is available any data to send */ if (sflow_counter_test(ð_counters)) { SFLADD_ELEMENT(cs, ð_elem); } So these counters will be sent only if there is indeed something available on the netdev level. Br, Robert _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
