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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
