On 12/11/23 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.

Hmm.  I'm not sure this is the only case.  For example, sFlow sampling
embeds packet header bytes into the sFlow header.  It should probably
contain correct checksums at this point.  There are maybe other cases
similar to this one as well.

Best regards, Ilya Maximets.

> 
> 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);
> +
>              *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)

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

Reply via email to