On 6/24/26 5:45 PM, David Marchand wrote:
> Use dynamic packet batches in the ip fragmentation code and simplify the
> code accordingly.
> Update the unit test that covers reassembly of large packets.
> 
> Signed-off-by: David Marchand <[email protected]>
> ---
> Changes since RFC v1:
> - used new helper for IP fragments,
> 
> ---
>  lib/ipf.c             | 42 ++++++++++++------------------------------
>  tests/ofproto-dpif.at | 42 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 53 insertions(+), 31 deletions(-)
> 
> diff --git a/lib/ipf.c b/lib/ipf.c
> index 551261d4e8..4a0f6fc2e5 100644
> --- a/lib/ipf.c
> +++ b/lib/ipf.c
> @@ -974,11 +974,9 @@ ipf_extract_frags_from_batch(struct ipf *ipf, struct 
> dp_packet_batch *pb,
>              if (!ipf_handle_frag(ipf, pkt, dl_type, zone, now, hash_basis,
>                                   &rp)) {
>                  dp_packet_batch_add(pb, pkt);
> -            } else {
> -                if (rp && pb->count != NETDEV_MAX_BURST) {
> -                    dp_packet_batch_add(pb, rp->pkt);
> -                    rp->list->reass_execute_ctx = rp->pkt;
> -                }
> +            } else if (rp) {
> +                dp_packet_batch_add(pb, rp->pkt);
> +                rp->list->reass_execute_ctx = rp->pkt;
>              }
>              ovs_mutex_unlock(&ipf->ipf_lock);
>          } else {
> @@ -1021,33 +1019,23 @@ ipf_purge_list_check(struct ipf *ipf, struct ipf_list 
> *ipf_list,
>  
>  /* Does the packet batch management and common accounting work associated
>   * with 'ipf_send_completed_frags()' and 'ipf_send_expired_frags()'. */
> -static bool
> +static void
>  ipf_send_frags_in_list(struct ipf *ipf, struct ipf_list *ipf_list,
>                         struct dp_packet_batch *pb, bool v6, long long now)
>      OVS_REQUIRES(ipf->ipf_lock)
>  {
>      if (ipf_purge_list_check(ipf, ipf_list, now)) {
> -        return true;
> +        return;
>      }
>  
>      while (ipf_list->last_sent_idx < ipf_list->last_inuse_idx) {
>          struct dp_packet *pkt
>              = ipf_list->frag_list[ipf_list->last_sent_idx + 1].pkt;
> -        if (pb->count != NETDEV_MAX_BURST) {
> -            dp_packet_batch_add(pb, pkt);
> -            ipf_list->last_sent_idx++;
> -            atomic_count_dec(&ipf->nfrag);
> -
> -            ipf_count(ipf, v6, IPF_NFRAGS_COMPL_SENT);
> -
> -            if (ipf_list->last_sent_idx == ipf_list->last_inuse_idx) {
> -                return true;
> -            }
> -        } else {
> -            return false;
> -        }
> +        dp_packet_batch_add(pb, pkt);
> +        atomic_count_dec(&ipf->nfrag);
> +        ipf_count(ipf, v6, IPF_NFRAGS_COMPL_SENT);
> +        ipf_list->last_sent_idx++;
>      }
> -    OVS_NOT_REACHED();
>  }
>  
>  /* Adds fragments associated with a completed fragment list to a packet batch
> @@ -1080,11 +1068,8 @@ ipf_send_completed_frags(struct ipf *ipf, struct 
> dp_packet_batch *pb,
>              continue;
>          }
>  
> -        if (ipf_send_frags_in_list(ipf, ipf_list, pb, v6, now)) {
> -            ipf_completed_list_clean(&ipf->frag_lists, ipf_list);
> -        } else {
> -            break;
> -        }
> +        ipf_send_frags_in_list(ipf, ipf_list, pb, v6, now);
> +        ipf_completed_list_clean(&ipf->frag_lists, ipf_list);
>      }
>  
>      ovs_mutex_unlock(&ipf->ipf_lock);
> @@ -1146,8 +1131,7 @@ ipf_execute_reass_pkts(struct ipf *ipf, struct 
> dp_packet_batch *pb,
>  
>      LIST_FOR_EACH_SAFE (rp, rp_list_node, &ipf->reassembled_pkt_list) {
>          if (!rp->list->reass_execute_ctx &&
> -            rp->list->key.dl_type == dl_type &&
> -            pb->count != NETDEV_MAX_BURST) {
> +            rp->list->key.dl_type == dl_type) {
>              dp_packet_batch_add(pb, rp->pkt);
>              rp->list->reass_execute_ctx = rp->pkt;
>          }
> @@ -1173,8 +1157,6 @@ ipf_post_execute_reass_pkts(struct ipf *ipf,
>          const size_t pb_cnt = dp_packet_batch_size(pb);
>          int pb_idx;
>          struct dp_packet *pkt;
> -        /* Inner batch loop is constant time since batch size is <=
> -         * NETDEV_MAX_BURST. */
>          DP_PACKET_BATCH_REFILL_FOR_EACH (pb_idx, pb_cnt, pkt, pb) {
>              if (rp && pkt == rp->list->reass_execute_ctx) {
>                  const struct ipf_frag *frag_0 = &rp->list->frag_list[0];
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 09bb478415..832a598862 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -5581,7 +5581,7 @@ 
> recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=l
>  dnl Clear ipf state.
>  ovs-appctl time/warp 33000
>  
> -dnl Send a lot of small frags to overload the dp packet batch max.
> +dnl Send a lot of small frags to trigger dynamic batch.
>  AT_CHECK([ovs-appctl dpctl/ipf-set-min-frag v4 420], [], [dnl
>  setting minimum fragment size successful
>  ])
> @@ -5592,14 +5592,22 @@ GEN_IP_FRAGS([p90], [22:22:22:22:22:22], 
> [11:11:11:11:11:11], [2.2.2.2], [1.1.1.
>  for i in $(seq 0 61); do
>      ovs-appctl netdev-dummy/receive p90 "$(cat p90_$i)"
>  done
> +OVS_CHECK_BATCH_GROW([0])
> +CHECK_COVERAGE([dpif_netdev_recirc_big_batch], [0])
>  AT_CHECK([ovs-appctl netdev-dummy/receive p90 "$(cat p90_62)"])
> +OVS_CHECK_BATCH_GROW([2])
> +CHECK_COVERAGE([dpif_netdev_recirc_big_batch], [1])
>  
>  dnl Second zone.
>  GEN_IP_FRAGS([p1], [11:11:11:11:11:11], [22:22:22:22:22:22], [1.1.1.1], 
> [2.2.2.2], [1], [2], [448])
>  for i in $(seq 0 61); do
>      ovs-appctl netdev-dummy/receive p1 "$(cat p1_$i)"
>  done
> +OVS_CHECK_BATCH_GROW([2])
> +CHECK_COVERAGE([dpif_netdev_recirc_big_batch], [1])
>  AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat p1_62)"])
> +OVS_CHECK_BATCH_GROW([4])
> +CHECK_COVERAGE([dpif_netdev_recirc_big_batch], [2])
>  
>  dnl Bump the remaining queued packets
>  AT_CHECK([ovs-appctl netdev-dummy/receive p90 "0021853763af 0026b98cb0f9 
> 0800 4500 04c4 0002 0096 40 06 af61 ac11370d ac11370b dnl
> @@ -5624,6 +5632,38 @@ 
> recirc(),in_port(90),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=l
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
>  
> +AT_SETUP([ofproto-dpif - fragment handling - reassembly no recirculation])
> +OVS_VSWITCHD_START
> +add_of_ports br0 1 2
> +AT_CHECK([ovs-vsctl set Interface p1 options:ol_ip_rx_csum_set_good=true])
> +
> +AT_DATA([flows.txt], [dnl
> +dnl Expand flow mask.
> +table=0 in_port=1,dl_src=55:55:55:55:55:55, actions=drop
> +table=0 in_port=1,ip,nw_src=10.0.0.2, actions=drop
> +table=0 in_port=1,ip,tcp actions=ct(commit, zone=10),output:2
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow11 replace-flows br0 flows.txt])
> +
> +dnl Send a lot of small frags to trigger dynamic batch.
> +AT_CHECK([ovs-appctl dpctl/ipf-set-min-frag v4 420], [], [dnl
> +setting minimum fragment size successful
> +])
> +
> +GEN_IP_FRAGS([p1], [11:11:11:11:11:11], [22:22:22:22:22:22], [1.1.1.1], 
> [2.2.2.2], [1], [2], [448])
> +
> +for i in $(seq 0 61); do
> +    ovs-appctl netdev-dummy/receive p1 "$(cat p1_$i)"

AT_CHECK

> +done
> +OVS_CHECK_BATCH_GROW([0])
> +CHECK_COVERAGE([dpif_netdev_output_grow_queues], [0])
> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$(cat p1_62)"])
> +OVS_CHECK_BATCH_GROW([2])
> +CHECK_COVERAGE([dpif_netdev_output_grow_queues], [1])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([ofproto-dpif - handling of malformed TCP packets])
>  OVS_VSWITCHD_START
>  add_of_ports br0 1 90

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to