On 3/22/23 00:41, Ilya Maximets wrote:
> On 3/15/23 07:07, Nobuhiro MIKI wrote:
>> This patch adds ODP actions for SRv6 and its tests.
>>
>> Signed-off-by: Nobuhiro MIKI <nm...@yahoo-corp.jp>
>> ---
>> lib/odp-util.c | 56 +++++++++++++++++++++++++++++++++++
>> python/ovs/flow/odp.py | 8 +++++
>> python/ovs/tests/test_odp.py | 16 ++++++++++
>> tests/odp.at | 1 +
>> tests/tunnel-push-pop-ipv6.at | 23 ++++++++++++++
>> 5 files changed, 104 insertions(+)
>>
>> diff --git a/lib/odp-util.c b/lib/odp-util.c
>> index dbd4554d0626..75f4d5242183 100644
>> --- a/lib/odp-util.c
>> +++ b/lib/odp-util.c
>> @@ -714,6 +714,24 @@ format_odp_tnl_push_header(struct ds *ds, struct
>> ovs_action_push_tnl *data)
>> ds_put_char(ds, ')');
>> }
>>
>> + ds_put_char(ds, ')');
>> + } else if (data->tnl_type == OVS_VPORT_TYPE_SRV6) {
>> + const struct srv6_base_hdr *srh;
>> + struct in6_addr *segs;
>> + int nr_segs;
>> + int i;
>> +
>> + srh = (const struct srv6_base_hdr *) l4;
>> + segs = ALIGNED_CAST(struct in6_addr *, srh + 1);
>> + nr_segs = srh->last_entry + 1;
>> +
>> + ds_put_format(ds, "srv6(");
>> + ds_put_format(ds, "segments_left=%d", srh->rt_hdr.segments_left);
>> + ds_put_format(ds, ",segs=");
>> + for (i = 0; i < nr_segs; i++) {
>> + ds_put_format(ds, i > 0 ? "," : "");
>> + ipv6_format_addr(&segs[nr_segs - i - 1], ds);
>> + }
>> ds_put_char(ds, ')');
>> } else if (data->tnl_type == OVS_VPORT_TYPE_GRE ||
>> data->tnl_type == OVS_VPORT_TYPE_IP6GRE) {
>> @@ -1534,6 +1552,7 @@ ovs_parse_tnl_push(const char *s, struct
>> ovs_action_push_tnl *data)
>> uint8_t hwid, dir;
>> uint32_t teid;
>> uint8_t gtpu_flags, gtpu_msgtype;
>> + uint8_t segments_left;
>>
>> if (!ovs_scan_len(s, &n, "tnl_push(tnl_port(%"SCNi32"),",
>> &data->tnl_port)) {
>> return -EINVAL;
>> @@ -1775,6 +1794,43 @@ ovs_parse_tnl_push(const char *s, struct
>> ovs_action_push_tnl *data)
>> tnl_type = OVS_VPORT_TYPE_GTPU;
>> header_len = sizeof *eth + ip_len +
>> sizeof *udp + sizeof *gtph;
>> + } else if (ovs_scan_len(s, &n, "srv6(segments_left=%"SCNu8,
>> + &segments_left)) {
>> + struct srv6_base_hdr *srh = (struct srv6_base_hdr *) (ip6 + 1);
>> + uint8_t n_segs = segments_left + 1;
>> + char seg_s[IPV6_SCAN_LEN + 1];
>> + struct in6_addr *segs;
>> + struct in6_addr seg;
>> +
>> + ip6->ip6_nxt = IPPROTO_ROUTING;
>> +
>> + srh->rt_hdr.type = IPV6_SRCRT_TYPE_4;
>> + srh->rt_hdr.segments_left = segments_left;
>> + srh->rt_hdr.hdrlen = 2 * n_segs;
>> + srh->last_entry = n_segs - 1;
>> +
>> + tnl_type = OVS_VPORT_TYPE_SRV6;
>> + header_len = sizeof *eth + ip_len +
>> + sizeof *srh + 8 * srh->rt_hdr.hdrlen;
>> +
>> + /* Parse segment list */
>> + if (!ovs_scan_len(s, &n, ",segs="IPV6_SCAN_FMT, seg_s)
>> + || inet_pton(AF_INET6, seg_s, &seg) != 1) {
>> + return -EINVAL;
>> + }
>> +
>> + segs = ALIGNED_CAST(struct in6_addr *, srh + 1);
>> + segs += n_segs - 1;
>> + memcpy(segs--, &seg, sizeof(struct in6_addr));
>> +
>> + while (ovs_scan_len(s, &n, ","IPV6_SCAN_FMT, seg_s)
>> + && inet_pton(AF_INET6, seg_s, &seg) == 1) {
>> + memcpy(segs--, &seg, sizeof(struct in_addr));
>
> We will overwrite the header here if segments_left + 1 is less
> than total number of specified segments. E.g. in the following
> case: srv6(segments_left=0,segs=2001:cafe::92,2001:cafe::93).
>
> One solution for this may be to create an SRV6_MAX_SEGS + 1 sized
> array and parse values there. Once all the values parsed,
> copy them into an actual header. If more than SRV6_MAX_SEGS
> values found, then fail the parsing.
>
> What do you think?
>
> Also, the sizeof seems incorrect here. It should be in6, not in.
> It also should be just 'sizeof *segs' to avoid any potential
> problems of this kind.
>
> Would be good to add a test to tests/odp.at for many segments
> parsing while segments_left is small.
>
> And it's probably possible to avoid some code duplication here.
> E.g. (didn't test):
>
> if (strncmp(s, ",segs=", 6)) {
> return -EINVAL;
> }
> s += 6;
We need to check &s[n] and advance n, of course. :/
> while (ovs_scan_len(s, &n, IPV6_SCAN_FMT, seg_s)
> && inet_pton(AF_INET6, seg_s, &seg) == 1) {
>
> ...
>
> if (s[n] == ',') {
> s++;
n++ here.
> }
> }
>
> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
