> This allows configuring hardware offload on/off.  The ct_sweep
> expiration is always trying to fill the ops batch anyway, so it
> doesn't need an actual check for enabled / disabled.  Wrapping that
> code in a check may also be harmful in the event that offload is
> disabled with offloaded connections.
>
> Assisted-by: Claude Sonnet 4.6 <[email protected]>
> Signed-off-by: Aaron Conole <[email protected]>

Hi Aaron,

The patch looks good to me. Some small nits below.

//Eelco

> ---
>  lib/conntrack.c    |  5 +++--
>  lib/ct-offload.c   | 46 +++++++++++++++++++++++++++++++++++++++++++---
>  lib/ct-offload.h   |  9 +++++++++
>  lib/dpif-offload.c | 16 ++++++++++++++++
>  lib/dpif-offload.h |  1 +
>  5 files changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index fbc7e43dc6..eb22039372 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1396,7 +1396,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
>          }
>          ovs_mutex_unlock(&ct->ct_lock);
>
> -        if (conn) {
> +        if (conn && ct_offload_enabled()) {

Should ct_offload_enabled() be checked first to
short-circuit the other conditions when offload is
disabled?

>              struct ct_offload_ctx offload_ctx = {
>                  .conn          = conn,
>                  .netdev_in     = NULL,
> @@ -1410,7 +1410,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
>      }
>
>      if (!create_new_conn && conn && ctx->reply &&
> -        (pkt->md.ct_state & CS_ESTABLISHED)) {
> +        (pkt->md.ct_state & CS_ESTABLISHED) &&
> +        ct_offload_enabled()) {

See above.

>          /* Notify offload providers that the connection is established.
>           * We use the reply bit to detect that the connection has
>           * transitioned and give us the input port, which should be the

[...]

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

Reply via email to