[ovs-dev,01/14] datapath-windows: Cleanup Actions.c

Message ID 20180709134056.7060-2-aserdean@ovn.org
State Changes Requested
Headers show
Series
  • Cleanup datapath-windows
Related show

Commit Message

Alin Gabriel Serdean July 9, 2018, 1:40 p.m.
Assign variables directly instead of reassigning them after.

Also purge unused variable `PNL_ATTR queueAttr`.

Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
---
 datapath-windows/ovsext/Actions.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

Comments

Shashank Ram July 11, 2018, 10:48 p.m. | #1
On 07/09/2018 06:40 AM, Alin Gabriel Serdean wrote:
> Assign variables directly instead of reassigning them after.
>
> Also purge unused variable `PNL_ATTR queueAttr`.
>
> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
> ---
>   datapath-windows/ovsext/Actions.c | 24 ++++++++++--------------
>   1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
> index 6922f0593..860d0abfa 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -152,11 +152,10 @@ OvsInitForwardingCtx(OvsForwardingContext *ovsFwdCtx,
>   static __inline VOID
>   OvsDoFragmentNbl(OvsForwardingContext *ovsFwdCtx, UINT16 mru)
>   {
> -    PNET_BUFFER_LIST fragNbl = NULL;
> -    fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,
> -                             ovsFwdCtx->curNbl,
> -                             &(ovsFwdCtx->layers),
> -                             mru, 0, TRUE);
> +    PNET_BUFFER_LIST fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,
> +                                              ovsFwdCtx->curNbl,
> +                                              &(ovsFwdCtx->layers),
> +                                              mru, 0, TRUE);
>   
>      if (fragNbl != NULL) {
>           OvsCompleteNBL(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl, TRUE);
> @@ -688,11 +687,11 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
>   
>           OvsCompleteNBLForwardingCtx(ovsFwdCtx,
>                                       L"Complete after cloning NBL for encapsulation");
> -        status = OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
> -                                      newNbl, srcVportNo, 0,
> -                                      NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
> -                                      ovsFwdCtx->completionList,
> -                                      &ovsFwdCtx->layers, FALSE);
> +        OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
> +                             newNbl, srcVportNo, 0,
> +                             NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
> +                             ovsFwdCtx->completionList,
> +                             &ovsFwdCtx->layers, FALSE);
>           ovsFwdCtx->curNbl = newNbl;
>           /* Update the forwarding detail for the new NBL */
>           ovsFwdCtx->fwdDetail->SourcePortId = srcPortId;
> @@ -1815,7 +1814,6 @@ OvsOutputUserspaceAction(OvsForwardingContext *ovsFwdCtx,
>   {
>       NTSTATUS status = NDIS_STATUS_SUCCESS;
>       PNL_ATTR userdataAttr;
> -    PNL_ATTR queueAttr;
>       POVS_PACKET_QUEUE_ELEM elem;
>       POVS_PACKET_HDR_INFO layers = &ovsFwdCtx->layers;
>       BOOLEAN isRecv = FALSE;
> @@ -1830,7 +1828,6 @@ OvsOutputUserspaceAction(OvsForwardingContext *ovsFwdCtx,
>           }
>       }
>   
> -    queueAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_PID);
>       userdataAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_USERDATA);
>   
>       elem = OvsCreateQueueNlPacket(NlAttrData(userdataAttr),
> @@ -2361,13 +2358,12 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
>   
>           OvsDeferredActionsLevelDec();
>       } else {
> -        POVS_VPORT_ENTRY vport = NULL;
> +        POVS_VPORT_ENTRY vport = OvsFindVportByPortNo(switchContext, srcPortNo);
>           LIST_ENTRY missedPackets;
>           UINT32 num = 0;
>   

Nit: Might be good to define uninitialized variables first, then the 
initialized ones.

>           ovsFwdCtx.switchContext->datapath.misses++;
>           InitializeListHead(&missedPackets);
> -        vport = OvsFindVportByPortNo(switchContext, srcPortNo);
>           if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) {
>               OvsCompleteNBLForwardingCtx(&ovsFwdCtx,
>                   L"OVS-Dropped due to port removal");

Acked-by: Shashank Ram <rams@vmware.com>

Patch

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 6922f0593..860d0abfa 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -152,11 +152,10 @@  OvsInitForwardingCtx(OvsForwardingContext *ovsFwdCtx,
 static __inline VOID
 OvsDoFragmentNbl(OvsForwardingContext *ovsFwdCtx, UINT16 mru)
 {
-    PNET_BUFFER_LIST fragNbl = NULL;
-    fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,
-                             ovsFwdCtx->curNbl,
-                             &(ovsFwdCtx->layers),
-                             mru, 0, TRUE);
+    PNET_BUFFER_LIST fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,
+                                              ovsFwdCtx->curNbl,
+                                              &(ovsFwdCtx->layers),
+                                              mru, 0, TRUE);
 
    if (fragNbl != NULL) {
         OvsCompleteNBL(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl, TRUE);
@@ -688,11 +687,11 @@  OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
 
         OvsCompleteNBLForwardingCtx(ovsFwdCtx,
                                     L"Complete after cloning NBL for encapsulation");
-        status = OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
-                                      newNbl, srcVportNo, 0,
-                                      NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
-                                      ovsFwdCtx->completionList,
-                                      &ovsFwdCtx->layers, FALSE);
+        OvsInitForwardingCtx(ovsFwdCtx, ovsFwdCtx->switchContext,
+                             newNbl, srcVportNo, 0,
+                             NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(newNbl),
+                             ovsFwdCtx->completionList,
+                             &ovsFwdCtx->layers, FALSE);
         ovsFwdCtx->curNbl = newNbl;
         /* Update the forwarding detail for the new NBL */
         ovsFwdCtx->fwdDetail->SourcePortId = srcPortId;
@@ -1815,7 +1814,6 @@  OvsOutputUserspaceAction(OvsForwardingContext *ovsFwdCtx,
 {
     NTSTATUS status = NDIS_STATUS_SUCCESS;
     PNL_ATTR userdataAttr;
-    PNL_ATTR queueAttr;
     POVS_PACKET_QUEUE_ELEM elem;
     POVS_PACKET_HDR_INFO layers = &ovsFwdCtx->layers;
     BOOLEAN isRecv = FALSE;
@@ -1830,7 +1828,6 @@  OvsOutputUserspaceAction(OvsForwardingContext *ovsFwdCtx,
         }
     }
 
-    queueAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_PID);
     userdataAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_USERDATA);
 
     elem = OvsCreateQueueNlPacket(NlAttrData(userdataAttr),
@@ -2361,13 +2358,12 @@  OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 
         OvsDeferredActionsLevelDec();
     } else {
-        POVS_VPORT_ENTRY vport = NULL;
+        POVS_VPORT_ENTRY vport = OvsFindVportByPortNo(switchContext, srcPortNo);
         LIST_ENTRY missedPackets;
         UINT32 num = 0;
 
         ovsFwdCtx.switchContext->datapath.misses++;
         InitializeListHead(&missedPackets);
-        vport = OvsFindVportByPortNo(switchContext, srcPortNo);
         if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) {
             OvsCompleteNBLForwardingCtx(&ovsFwdCtx,
                 L"OVS-Dropped due to port removal");