[ovs-dev] [PATCH] ovs-hyperv: make kernel return values netlink socket like
Nithin Raju
nithin at vmware.com
Tue Apr 28 14:38:51 UTC 2015
hi Sorin,
Thanks so much for the review.
Let me address your comments and then send out the updated patch. If I missed anything, pls. let me know.
> UINT8 version;
> - UINT8 pad;
> + UINT8 pad1;
> UINT16 maxAttr;
> + UINT16 pad2;
> SV: Do we need the new 'pad' members, 'pad1' & 'pad2'? I didn't saw them referenced in any part of the code.
I used them to explicitly add padding in the structure to avoid holes.
> NETLINK_CMD *cmds; /* Array of netlink commands and handlers. */
> UINT16 opsCount;
> } NETLINK_FAMILY, *PNETLINK_FAMILY;
> @@ -143,12 +144,12 @@ NETLINK_CMD nlControlFamilyCmdOps[] = {
> },
> { .cmd = OVS_CTRL_CMD_EVENT_NOTIFY,
> .handler = OvsReadEventCmdHandler,
> - .supportedDevOp = OVS_READ_EVENT_DEV_OP,
> + .supportedDevOp = OVS_READ_DEV_OP,
> .validateDpIndex = FALSE,
> },
> { .cmd = OVS_CTRL_CMD_READ_NOTIFY,
> .handler = OvsReadPacketCmdHandler,
> - .supportedDevOp = OVS_READ_PACKET_DEV_OP,
> SV: The two operations, 'OVS_READ_EVENT_DEV_OP' and 'OVS_READ_PACKET_DEV_OP' are not used anymore. Shouldn't we remove them from the header as well?
I’ve removed it in the header file later.
> /*
> * --------------------------------------------------------------------------
> - * Function to invoke the netlink command handler.
> + * Function to invoke the netlink command handler. The function also
> + massages
> + * the return value of the handler function to construct a 'NL_ERROR'
> + message
> SV: Please rephrase the function description above - 'massage' is not appropriate.
Sure. I’ll do that.
-- Nithin
More information about the dev
mailing list