[ovs-dev] [PATCH 2/2] datapath-windows: use the Netlink set API and need new APIs

Nithin Raju nithin at vmware.com
Mon Sep 15 07:24:12 UTC 2014


On Sep 14, 2014, at 8:36 PM, Samuel Ghinet <sghinet at cloudbasesolutions.com>
 wrote:

> Hello Nithin,
> 
> Overall, it looks ok.
> 
> Very minor things, though:
> (OvsDpFillInfo)
> o) could you please rename nlWrite into something more boolean-like, like "ok" or whatever you prefer?
> When I first read that piece of code, I was thinking nlWrite is a ptr returned by NlMsgPutHead. It confused me for a few minutes.
> 
> o) the variables datapath and dpStats are only used in the second if branch. I think it would look better and cleaner if you declared them in that if branch.
> 
> o) Also, a very minor thing, I think it would be nice if we could make a typedef in, e.g., OvsDpInterfaceExt.h:
> typedef struct ovs_dp_stats OVS_DP_STATS;
> it would make the code in the windows driver more consistent with all the other parts.

Thanks for the comments. I'll address all of them in the v2 patch.

Thanks,
-- Nithin




More information about the dev mailing list