ofproto-dpif-xlate: Remove repeated function for garp.

Function is_gratuitous_arp() and function is_garp() are all used to judge whether the flow is gratuitous arp. It is not necessary to use two functions to do the same thing and just keep one. Gratuitous ARP message is a generally link-level broadcast messages and carry the same IP in sender and target fields. This patch add the check in function is_garp() whether the arp is a broadcast message and whether the arp is an arp request or an arp reply. Signed-off-by: Han Ding <[email protected]> Signed-off-by: 0-day Robot <[email protected]>
ovsrobot · Jan 8, 2025 · 9395c3c · 9395c3c
1 parent b72f6bb
commit 9395c3c
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 29 deletions.
diff --git a/lib/flow.h b/lib/flow.h
@@ -1143,10 +1143,23 @@ static inline bool is_garp(const struct flow *flow,
                            struct flow_wildcards *wc)
 {
     if (is_arp(flow)) {
-        return (FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_src) ==
-                FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_dst));
-    }
+        if (wc) {
+            WC_MASK_FIELD(wc, dl_dst);
+        }
+
+        if (!eth_addr_is_broadcast(flow->dl_dst)) {
+            return false;
+        }
 
+        if (wc) {
+            WC_MASK_FIELD(wc, nw_proto);
+        }
+
+        if (flow->nw_proto == ARP_OP_REQUEST) {
+            return (FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_src) ==
+                    FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_dst));
+        }
+    }
     return false;
 }
 

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
@@ -2693,30 +2693,6 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
 /* A VM broadcasts a gratuitous ARP to indicate that it has resumed after
  * migration.  Older Citrix-patched Linux DomU used gratuitous ARP replies to
  * indicate this; newer upstream kernels use gratuitous ARP requests. */
-static bool
-is_gratuitous_arp(const struct flow *flow, struct flow_wildcards *wc)
-{
-    if (flow->dl_type != htons(ETH_TYPE_ARP)) {
-        return false;
-    }
-
-    memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
-    if (!eth_addr_is_broadcast(flow->dl_dst)) {
-        return false;
-    }
-
-    memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
-    if (flow->nw_proto == ARP_OP_REPLY) {
-        return true;
-    } else if (flow->nw_proto == ARP_OP_REQUEST) {
-        memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
-        memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
-
-        return flow->nw_src == flow->nw_dst;
-    } else {
-        return false;
-    }
-}
 
 /* Determines whether packets in 'flow' within 'xbridge' should be forwarded or
  * dropped.  Returns true if they may be forwarded, false if they should be
@@ -2766,7 +2742,7 @@ is_admissible(struct xlate_ctx *ctx, struct xport *in_port,
             mac = mac_learning_lookup(xbridge->ml, flow->dl_src, vlan);
             if (mac
                 && mac_entry_get_port(xbridge->ml, mac) != in_xbundle->ofbundle
-                && (!is_gratuitous_arp(flow, ctx->wc)
+                && (!is_garp(flow, ctx->wc)
                     || mac_entry_is_grat_arp_locked(mac))) {
                 ovs_rwlock_unlock(&xbridge->ml->rwlock);
                 xlate_report(ctx, OFT_DETAIL,
@@ -3214,7 +3190,7 @@ xlate_normal(struct xlate_ctx *ctx)
     }
 
     /* Learn source MAC. */
-    bool is_grat_arp = is_gratuitous_arp(flow, wc);
+    bool is_grat_arp = is_garp(flow, wc);
     if (ctx->xin->allow_side_effects
         && flow->packet_type == htonl(PT_ETH)
         && in_port && in_port->pt_mode != NETDEV_PT_LEGACY_L3