On Sun, Dec 04, 2011 at 04:57:19PM -0800, Justin Pettit wrote:
> Mike Bursell pointed out that our policer only works on IPv4
> traffic--and specifically not IPv6.  By using the "basic" filter, we can
> enforce policing on all traffic for a particular interface.
> 
> Jamal Hadi Salim pointed out that calling "tc" directly with system() is
> pretty ugly.  This commit switches our remaining "tc" calls to directly
> sending the appropriate netlink messages.
> 
> Suggested-by: Mike Bursell <mike.burs...@citrix.com>
> Suggested-by: Jamal Hadi Salim <h...@cyberus.ca>

I assume you tested it, I only built it.

"sparse" says:

    ../lib/netdev-linux.c:3547:47: warning: incorrect type in argument 2
    (different base types)
    ../lib/netdev-linux.c:3547:47:    expected unsigned int [unsigned]
    minor
    ../lib/netdev-linux.c:3547:47:    got restricted __be16

That's about this line:
    tcmsg->tcm_info = tc_make_handle(49, htons(ETH_P_ALL));
This is a "false positive" so we need to placate sparse with:
    tcmsg->tcm_info = tc_make_handle(49, (OVS_FORCE uint16_t) htons(ETH_P_ALL));

Also, netdev_linux_set_policing() introduces a duplicate netdev_name
in an inner block.  You can just delete it, the outer block's
definition is identical.

There's a missing space before ':' here:
+    netdev_dev->kbits_burst = kbits_rate ? kbits_burst: 0;
But I think you could just write 'kbits_burst' by itself without any
?: because we normalize its value at the top of the function.

In tc_add_policer(), please put spaces around the | operator.  I find
that it is easier to read that way.

Otherwise, this looks good.

Thank you, this is a real improvement.

Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to