On 4/15/25 12:24 PM, yanxia wrote:
> Hello:
> 
> This patch  aims to enhance the functionality of the tunnel_port
> by enabling the flow-based setting of TTL and TOS.
> This improvement provides more flexibility in network traffic
> management and allows for more granular control over tunneled
> connections.In our specific application scenario, the dynamic
> adjustment of TTL and TOS based on flows is of utmost importance.
> There are various situations where such dynamic settings are
> required. For example, in a multi-tenant cloud environment,
> different tenants may have distinct SLAs. High-priority tenants
> may demand lower latency and higher throughput for their
> network traffic.
> 
> This is the second version of the patch.
> - Fixed the build failure issue caused by typo: resolve the typo
> of function call from ds_put_cstr to ds_put_char.
> 
> Signed-off-by: yanxia <yana06...@163.com>
> ---
>  include/openvswitch/meta-flow.h |  5 +++--
>  lib/meta-flow.xml               |  5 +++--
>  lib/netdev-vport.c              |  9 +++++++++
>  lib/netdev.h                    |  3 +++
>  ofproto/tunnel.c                | 29 +++++++++++++++++++----------
>  5 files changed, 37 insertions(+), 14 deletions(-)

Hi, yanxia.  Thanks for the patch!

I think, the idea is reasonable, but the patch is missing a few parts, 
primarily,
it is missing exposure of these new fields to OpenFlow including documentation
and it's missing tests that would cover this new functionality from both 
OpenFlow
side - checking that these fields are properly reported as supported in 
appropriate
OpenFlow versions and are properly parsed in OpenFlow messages, and the actual
functionality tests that would check that these fields are properly set in the
datapath flows.

A few general formatting comments:
1. The v2 shouldn't be at the end of the subject line.
2. As the bot pointed out, the name of the patch should end with a dot.
   You may check the patch before sending by running utilities/checkpatch.py
   script on your git repo.
3. 'Hello:' should be removed form the commit message as it should not end up
   in the git.
4. There is an unrelated empty line change in the end of the patch.
5. If possible, provide you name in the Firstname Lastname <email@domain> format
   for the DCO purposes as documented here:
      https://docs.openvswitch.org/en/latest/faq/contributing/

> 
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index 875f122c5..5f48bfd12 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -401,7 +401,7 @@ enum OVS_PACKED_ENUM mf_field_id {
>       * Maskable: no.
>       * Formatting: decimal.
>       * Prerequisites: none.
> -     * Access: read-only.
> +     * Access: read/write.
>       * NXM: none.
>       * OXM: none.

As an example of what is missing, you should populate the description above with
exact versions where these new fields are available via OXM or NXM.  I assume 
you
exported them through NXM, right?  This way they will be properly exposed in the
auto-generated documentation.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to