Looks good but for a few comments.

>+static __inline NDIS_STATUS
>+OvsCtFlush(UINT16 zone)
>+{
>+    PLIST_ENTRY link, next;
>+    POVS_CT_ENTRY entry;
>+
>+    LOCK_STATE_EX lockState;
>+    NdisAcquireRWLockWrite(ovsConntrackLockObj, &lockState, 0);
>+
>+    for (int i = 0; i < CT_HASH_TABLE_SIZE; i++) {
>+        LIST_FORALL_SAFE(&ovsConntrackTable[i], link, next) {
>+            entry = CONTAINING_RECORD(link, OVS_CT_ENTRY, link);
>+            if (!zone || zone == entry->key.zone)

Is 0 a valid value for zone? If yes, we¹ll end up deleting all the entries
when zone == 0.


>+    nlmsgType = (NFNL_SUBSYS_CTNETLINK << 8 | IPCTNL_MSG_CT_DELETE);
>+    NlBufInit(&nlBuf,
>+              usrParamsCtx->outputBuffer,
>+              usrParamsCtx->outputLength);
>+    status = NlFillOvsMsgForNfGenMsg(&nlBuf, nlmsgType, NLM_F_CREATE,
>+                                     msgIn->nlMsg.nlmsgSeq,
>+                                     msgIn->nlMsg.nlmsgPid,
>+                                     AF_UNSPEC,
>+                                     msgIn->nfGenMsg.version,
>+                                     gOvsSwitchContext->dpNo);

'gOvsSwitchContext->dpNo¹ is not part of the struct nlhdr + struct
nfgenmsg combination. You¹ll have to decrement the size of the output by
Œsizeof gOvsSwitchContext->dpNo¹.


Also, if there¹s an error, you¹ll end up writing to the output buffer
twice. You should probably structure the code as:

status = OvsCtFlush(zone);
if (status == STATUS_SUCCESS) {
    /* fill success message. */
    *replyLen = blah;
}

done:
    nlError = NlMapStatusToNlErr(status);

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

Reply via email to