On 5/22/23 10:17, Nobuhiro MIKI wrote: > On 2023/05/22 12:09, Nobuhiro MIKI wrote: >> On 2023/05/20 3:56, Ilya Maximets wrote: >>> On 5/16/23 07:33, 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 <[email protected]> >>>> --- >>>> lib/netdev-native-tnl.c | 23 +++++++++++++---------- >>>> lib/netdev-native-tnl.h | 4 ++-- >>>> lib/packets.c | 2 +- >>>> lib/packets.h | 2 ++ >>>> 4 files changed, 18 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c >>>> index 9abdf51076a8..db1c4c6d9bfc 100644 >>>> --- a/lib/netdev-native-tnl.c >>>> +++ b/lib/netdev-native-tnl.c >>>> @@ -146,8 +146,8 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, >>>> struct flow_tnl *tnl, >>>> * >>>> * Return pointer to the L4 header added to 'packet'. */ >>>> void * >>>> -netdev_tnl_push_ip_header(struct dp_packet *packet, >>>> - const void *header, int size, int *ip_tot_size) >>>> +netdev_tnl_push_ip_header(struct dp_packet *packet, const void *header, >>>> + int size, int *ip_tot_size, ovs_be32 ipv6_label) >>>> { >>>> struct eth_header *eth; >>>> struct ip_header *ip; >>>> @@ -166,6 +166,7 @@ netdev_tnl_push_ip_header(struct dp_packet *packet, >>>> ip6 = netdev_tnl_ipv6_hdr(eth); >>>> *ip_tot_size -= IPV6_HEADER_LEN; >>>> ip6->ip6_plen = htons(*ip_tot_size); >>>> + packet_set_ipv6_flow_label(&ip6->ip6_flow, ipv6_label); >>> >>> This function is on a hot path. It might make sense to update >>> the flow label only if it is non-zero, to save some cycles. >>> >> >> Hi Ilya, >> >> Thanks for your review. Yes, I agree. >> I will simply insert an if block as follows: >> >> - packet_set_ipv6_flow_label(&ip6->ip6_flow, ipv6_label); >> + if (ipv6_label) { >> + packet_set_ipv6_flow_label(&ip6->ip6_flow, ipv6_label); >> + } >> > > If the implementation is to write an enum value in data->header at > header build and replace it with the hash value at header push, > I think it cannot be replaced by "if (ipv6_label) { ... }" block. > > The reason is that there are cases where an enum value is replaced > by 0 in the cases srv6_flowlabel="copy" and srv6_flowlabel="zero". > This would occur even if the order of the enum definitions is swapped. > Please correct me if my understanding is incorrect.
Yeah, you're right. Keep it as-is then. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
