On 11 Dec 2023, at 16:39, Mike Pattrick wrote:

> Currently dp_netdev_upcall() resolves checksums on all packets, but this
> isn't strictly needed. The checksums will be resolved before
> transmission. However, we do have to resolve the checksums before
> sending a packet to the controller as offload flags aren't retained.
>
> Signed-off-by: Mike Pattrick <m...@redhat.com>
> ---
>  lib/dpif-netdev.c             |  2 --
>  ofproto/ofproto-dpif-upcall.c | 13 ++++++++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9a59a1b03..8f7aaffbb 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -7980,8 +7980,6 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread *pmd, 
> struct dp_packet *packet_,
>          ds_destroy(&ds);
>      }
>
> -    dp_packet_ol_send_prepare(packet_, 0);
> -
>      return dp->upcall_cb(packet_, flow, ufid, pmd->core_id, type, userdata,
>                           actions, wc, put_actions, dp->upcall_aux);
>  }
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index cc10f57b5..e974d844a 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1574,15 +1574,20 @@ process_upcall(struct udpif *udpif, struct upcall 
> *upcall,
>              const struct frozen_state *state = &recirc_node->state;
>
>              struct ofproto_async_msg *am = xmalloc(sizeof *am);
> +
> +            /* Resolve offloaded checksums, if any. */
> +            struct dp_packet *packet_clone = dp_packet_clone(packet);
> +            dp_packet_ol_send_prepare(packet_clone, 0);

Doing a clone (malloc/copy), and doing another malloc and copy below sound like 
a waste of resources.

Can we maybe make this more smart in the dp_netdev_upcall()?
Something like the below in dp_netdev_upcall():

  if (type == CONTROLLER_UPCALL)
    dp_packet_ol_send_prepare(packet_, 0);


>              *am = (struct ofproto_async_msg) {
>                  .controller_id = cookie->controller.controller_id,
>                  .oam = OAM_PACKET_IN,
>                  .pin = {
>                      .up = {
>                          .base = {
> -                            .packet = xmemdup(dp_packet_data(packet),
> -                                              dp_packet_size(packet)),
> -                            .packet_len = dp_packet_size(packet),
> +                            .packet = xmemdup(dp_packet_data(packet_clone),
> +                                              dp_packet_size(packet_clone)),
> +                            .packet_len = dp_packet_size(packet_clone),
>                              .reason = cookie->controller.reason,
>                              .table_id = state->table_id,
>                              .cookie = get_32aligned_be64(
> @@ -1598,6 +1603,8 @@ process_upcall(struct udpif *udpif, struct upcall 
> *upcall,
>                  },
>              };
>
> +            dp_packet_delete(packet_clone);
> +
>              if (cookie->controller.continuation) {
>                  am->pin.up.stack = (state->stack_size
>                            ? xmemdup(state->stack, state->stack_size)
> -- 
> 2.39.3
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to