>On 2/7/23 06:04, Han Ding wrote:
>>
>> 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.
>>
>> Signed-off-by: Han Ding <[email protected]>
>> ---
>> lib/flow.h | 15 +++++++++++++--
>> ofproto/ofproto-dpif-xlate.c | 32 ++------------------------------
>> 2 files changed, 15 insertions(+), 32 deletions(-)
>>
>> diff --git a/lib/flow.h b/lib/flow.h
>> index c647ad83c..5b37543e0 100644
>> --- a/lib/flow.h
>> +++ b/lib/flow.h
>> @@ -1133,8 +1133,19 @@ 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 (!eth_addr_is_broadcast(FLOW_WC_GET_AND_MASK_WC(flow, wc,
>> dl_dst))) {
>> + return false;
>> + }
>> +
>> + if (wc) {
>> + memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
>
>WC_MASK_FIELD() macro should be used instead of memset.
Thanks for review. I will fix it in next version.
>
>> + }
>> +
>> + if (flow->nw_proto == ARP_OP_REQUEST ||
>> + flow->nw_proto == ARP_OP_REPLY) {
>> + 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
>> index a9cf3cbee..b3c13f6bf 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -2543,34 +2543,6 @@ output_normal(struct xlate_ctx *ctx, const struct
>> xbundle *out_xbundle,
>> memcpy(&ctx->xin->flow.vlans, &old_vlans, sizeof(old_vlans));
>> }
>>
>> -/* 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
>> * dropped.
>> @@ -2619,7 +2591,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,
>> @@ -3062,7 +3034,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
>> --
>> 2.27.0
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
