diff mbox series

[ovs-dev,v3,1/2] netdev-native-tnl: Add ipv6_label param in netdev_tnl_ip_build_header.

Message ID 20230509093800.33596-2-nmiki@yahoo-corp.jp
State Changes Requested
Headers show
Series Support flowlabel calculation in SRv6 tunnels | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Nobuhiro MIKI May 9, 2023, 9:37 a.m. UTC 
For tunnels such as SRv6, some popular vendor appliances support
IPv6 flowlabel based load balancing. In preparation for OVS to
support it, this patch modifies the encapsulation to allow IPv6
flowlabel to be configured.

Signed-off-by: Nobuhiro MIKI <nmiki@yahoo-corp.jp>
---
 lib/netdev-native-tnl.c | 14 ++++++++------
 lib/netdev-native-tnl.h |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

Comments

Ilya Maximets May 10, 2023, 8:56 p.m. UTC | #1 
On 5/9/23 11:37, Nobuhiro MIKI wrote:
> For tunnels such as SRv6, some popular vendor appliances support
> IPv6 flowlabel based load balancing. In preparation for OVS to
> support it, this patch modifies the encapsulation to allow IPv6
> flowlabel to be configured.
> 
> Signed-off-by: Nobuhiro MIKI <nmiki@yahoo-corp.jp>
> ---
>  lib/netdev-native-tnl.c | 14 ++++++++------
>  lib/netdev-native-tnl.h |  2 +-
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index 9abdf51076a8..55e1bd567fa1 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -35,6 +35,7 @@
>  #include "byte-order.h"
>  #include "csum.h"
>  #include "dp-packet.h"
> +#include "flow.h"
>  #include "netdev.h"
>  #include "netdev-vport.h"
>  #include "netdev-vport-private.h"
> @@ -276,7 +277,7 @@ eth_build_header(struct ovs_action_push_tnl *data,
>  void *
>  netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
>                             const struct netdev_tnl_build_header_params *params,
> -                           uint8_t next_proto)
> +                           uint8_t next_proto, uint32_t ipv6_label)
>  {
>      void *l3;
>  
> @@ -308,7 +309,8 @@ netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
>          ip6 = (struct ovs_16aligned_ip6_hdr *) l3;
>  
>          put_16aligned_be32(&ip6->ip6_flow, htonl(6 << 28) |
> -                           htonl(params->flow->tunnel.ip_tos << 20));
> +                           htonl(params->flow->tunnel.ip_tos << 20) |
> +                           htonl(ipv6_label & 0xfffff));

IPV6_LABEL_MASK should be used instead.  Also, it's a bit strange to
pass areound the lable in the host order.  Having it in ovs_be32
will likely be more intuitive.

But see the comments for the second patch.

Best regards, Ilya Maximets.
Nobuhiro MIKI May 11, 2023, 9:58 a.m. UTC | #2 
On 2023/05/11 5:56, Ilya Maximets wrote:
> On 5/9/23 11:37, Nobuhiro MIKI wrote:
>> For tunnels such as SRv6, some popular vendor appliances support
>> IPv6 flowlabel based load balancing. In preparation for OVS to
>> support it, this patch modifies the encapsulation to allow IPv6
>> flowlabel to be configured.
>>
>> Signed-off-by: Nobuhiro MIKI <nmiki@yahoo-corp.jp>
>> ---
>>  lib/netdev-native-tnl.c | 14 ++++++++------
>>  lib/netdev-native-tnl.h |  2 +-
>>  2 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
>> index 9abdf51076a8..55e1bd567fa1 100644
>> --- a/lib/netdev-native-tnl.c
>> +++ b/lib/netdev-native-tnl.c
>> @@ -35,6 +35,7 @@
>>  #include "byte-order.h"
>>  #include "csum.h"
>>  #include "dp-packet.h"
>> +#include "flow.h"
>>  #include "netdev.h"
>>  #include "netdev-vport.h"
>>  #include "netdev-vport-private.h"
>> @@ -276,7 +277,7 @@ eth_build_header(struct ovs_action_push_tnl *data,
>>  void *
>>  netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
>>                             const struct netdev_tnl_build_header_params *params,
>> -                           uint8_t next_proto)
>> +                           uint8_t next_proto, uint32_t ipv6_label)
>>  {
>>      void *l3;
>>  
>> @@ -308,7 +309,8 @@ netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
>>          ip6 = (struct ovs_16aligned_ip6_hdr *) l3;
>>  
>>          put_16aligned_be32(&ip6->ip6_flow, htonl(6 << 28) |
>> -                           htonl(params->flow->tunnel.ip_tos << 20));
>> +                           htonl(params->flow->tunnel.ip_tos << 20) |
>> +                           htonl(ipv6_label & 0xfffff));
> 
> IPV6_LABEL_MASK should be used instead.  Also, it's a bit strange to
> pass areound the lable in the host order.  Having it in ovs_be32
> will likely be more intuitive.
> 
> But see the comments for the second patch.

Hi Ilya,

Thanks for your review.

I'll replace 0xfffff with IPV6_LABEL_MASK and fix the byte order.
Based on the comments on the second patch, this code will be moved
to netdev_tnl_push_ip_header().

Best Regards,
Nobuhiro MIKI
diff mbox series

Patch

diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index 9abdf51076a8..55e1bd567fa1 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -35,6 +35,7 @@ 
 #include "byte-order.h"
 #include "csum.h"
 #include "dp-packet.h"
+#include "flow.h"
 #include "netdev.h"
 #include "netdev-vport.h"
 #include "netdev-vport-private.h"
@@ -276,7 +277,7 @@  eth_build_header(struct ovs_action_push_tnl *data,
 void *
 netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
                            const struct netdev_tnl_build_header_params *params,
-                           uint8_t next_proto)
+                           uint8_t next_proto, uint32_t ipv6_label)
 {
     void *l3;
 
@@ -308,7 +309,8 @@  netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
         ip6 = (struct ovs_16aligned_ip6_hdr *) l3;
 
         put_16aligned_be32(&ip6->ip6_flow, htonl(6 << 28) |
-                           htonl(params->flow->tunnel.ip_tos << 20));
+                           htonl(params->flow->tunnel.ip_tos << 20) |
+                           htonl(ipv6_label & 0xfffff));
         ip6->ip6_hlim = params->flow->tunnel.ip_ttl;
         ip6->ip6_nxt = next_proto;
         memcpy(&ip6->ip6_src, params->s_ip, sizeof(ovs_be32[4]));
@@ -326,7 +328,7 @@  udp_build_header(struct netdev_tunnel_config *tnl_cfg,
 {
     struct udp_header *udp;
 
-    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP);
+    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP, 0);
     udp->udp_dst = tnl_cfg->dst_port;
 
     if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
@@ -488,7 +490,7 @@  netdev_gre_build_header(const struct netdev *netdev,
     ovs_mutex_lock(&dev->mutex);
     tnl_cfg = &dev->tnl_cfg;
 
-    greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE);
+    greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE, 0);
 
     if (params->flow->packet_type == htonl(PT_ETH)) {
         greh->protocol = htons(ETH_TYPE_TEB);
@@ -644,7 +646,7 @@  netdev_erspan_build_header(const struct netdev *netdev,
     /* XXX: RCUfy tnl_cfg. */
     ovs_mutex_lock(&dev->mutex);
     tnl_cfg = &dev->tnl_cfg;
-    greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE);
+    greh = netdev_tnl_ip_build_header(data, params, IPPROTO_GRE, 0);
     ersh = ERSPAN_HDR(greh);
 
     tun_id = ntohl(be64_to_be32(params->flow->tunnel.tun_id));
@@ -880,7 +882,7 @@  netdev_srv6_build_header(const struct netdev *netdev,
         goto out;
     }
 
-    srh = netdev_tnl_ip_build_header(data, params, IPPROTO_ROUTING);
+    srh = netdev_tnl_ip_build_header(data, params, IPPROTO_ROUTING, 0);
     srh->rt_hdr.segments_left = nr_segs - 1;
     srh->rt_hdr.type = IPV6_SRCRT_TYPE_4;
     srh->rt_hdr.hdrlen = 2 * nr_segs;
diff --git a/lib/netdev-native-tnl.h b/lib/netdev-native-tnl.h
index 4dad8f978cc6..b06e7bbf2a72 100644
--- a/lib/netdev-native-tnl.h
+++ b/lib/netdev-native-tnl.h
@@ -118,7 +118,7 @@  netdev_tnl_ipv6_hdr(void *eth)
 void *
 netdev_tnl_ip_build_header(struct ovs_action_push_tnl *data,
                            const struct netdev_tnl_build_header_params *params,
-                           uint8_t next_proto);
+                           uint8_t next_proto, uint32_t ipv6_label);
 
 extern uint16_t tnl_udp_port_min;
 extern uint16_t tnl_udp_port_max;