On Dec 5, 2011, at 11:17 AM, Ben Pfaff wrote: > I assume you tested it, I only built it.
Yes. > "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)); Thanks. Fixed. > 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. Whoops. I introduced that in some last minute refactoring last night. > 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. Good point. > In tc_add_policer(), please put spaces around the | operator. I find > that it is easier to read that way. No problem. > Otherwise, this looks good. > > Thank you, this is a real improvement. Thanks for the review. I've pushed it with these changes. --Justin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
