Skip to content

Commit 7389074

Browse files
davem330davem330
committed
Merge branch 'ioam-fixes'
Justin Iurman says: ==================== Correct the IOAM behavior for undefined trace type bits (@jakub @david: there will be a conflict for #2 when merging net->net-next, due to commit [1]. The conflict is only 5-10 lines for #2 (#1 should be fine) inside the file tools/testing/selftests/net/ioam6.sh, so quite short though possibly ugly. Sorry for that, I didn't expect to post this one... Had I known, I'd have made the opposite.) Modify both the input and output behaviors regarding the trace type when one of the undefined bits is set. The goal is to keep the interoperability when new fields (aka new bits inside the range 12-21) will be defined. The draft [2] says the following: --------------------------------------------------------------- "Bit 12-21 Undefined. These values are available for future assignment in the IOAM Trace-Type Registry (Section 8.2). Every future node data field corresponding to one of these bits MUST be 4-octets long. An IOAM encapsulating node MUST set the value of each undefined bit to 0. If an IOAM transit node receives a packet with one or more of these bits set to 1, it MUST either: 1. Add corresponding node data filled with the reserved value 0xFFFFFFFF, after the node data fields for the IOAM-Trace-Type bits defined above, such that the total node data added by this node in units of 4-octets is equal to NodeLen, or 2. Not add any node data fields to the packet, even for the IOAM-Trace-Type bits defined above." --------------------------------------------------------------- The output behavior has been modified to respect the fact that "an IOAM encap node MUST set the value of each undefined bit to 0" (i.e., undefined bits can't be set anymore). As for the input behavior, current implementation is based on the second choice (i.e., "not add any data fields to the packet [...]"). With this solution, any interoperability is lost (i.e., if a new bit is defined, then an "old" kernel implementation wouldn't fill IOAM data when such new bit is set inside the trace type). The input behavior is therefore relaxed and these undefined bits are now allowed to be set. It is only possible thanks to the sentence "every future node data field corresponding to one of these bits MUST be 4-octets long". Indeed, the default empty value (the one for 4-octet fields) is inserted whenever an undefined bit is set. [1] cfbe9b0 [2] https://datatracker.ietf.org/doc/html/draft-ietf-ippm-ioam-data#section-5.4.1 ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents ef1100e + 7b1700e commit 7389074

File tree

4 files changed

+148
-118
lines changed

4 files changed

+148
-118
lines changed

‎net/ipv6/ioam6.c

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,66 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
770770
data += sizeof(__be32);
771771
}
772772

773+
/* bit12 undefined: filled with empty value */
774+
if (trace->type.bit12) {
775+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
776+
data += sizeof(__be32);
777+
}
778+
779+
/* bit13 undefined: filled with empty value */
780+
if (trace->type.bit13) {
781+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
782+
data += sizeof(__be32);
783+
}
784+
785+
/* bit14 undefined: filled with empty value */
786+
if (trace->type.bit14) {
787+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
788+
data += sizeof(__be32);
789+
}
790+
791+
/* bit15 undefined: filled with empty value */
792+
if (trace->type.bit15) {
793+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
794+
data += sizeof(__be32);
795+
}
796+
797+
/* bit16 undefined: filled with empty value */
798+
if (trace->type.bit16) {
799+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
800+
data += sizeof(__be32);
801+
}
802+
803+
/* bit17 undefined: filled with empty value */
804+
if (trace->type.bit17) {
805+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
806+
data += sizeof(__be32);
807+
}
808+
809+
/* bit18 undefined: filled with empty value */
810+
if (trace->type.bit18) {
811+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
812+
data += sizeof(__be32);
813+
}
814+
815+
/* bit19 undefined: filled with empty value */
816+
if (trace->type.bit19) {
817+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
818+
data += sizeof(__be32);
819+
}
820+
821+
/* bit20 undefined: filled with empty value */
822+
if (trace->type.bit20) {
823+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
824+
data += sizeof(__be32);
825+
}
826+
827+
/* bit21 undefined: filled with empty value */
828+
if (trace->type.bit21) {
829+
*(__be32 *)data = cpu_to_be32(IOAM6_U32_UNAVAILABLE);
830+
data += sizeof(__be32);
831+
}
832+
773833
/* opaque state snapshot */
774834
if (trace->type.bit22) {
775835
if (!sc) {
@@ -791,16 +851,10 @@ void ioam6_fill_trace_data(struct sk_buff *skb,
791851
struct ioam6_schema *sc;
792852
u8 sclen = 0;
793853

794-
/* Skip if Overflow flag is set OR
795-
* if an unknown type (bit 12-21) is set
854+
/* Skip if Overflow flag is set
796855
*/
797-
if (trace->overflow ||
798-
trace->type.bit12 | trace->type.bit13 | trace->type.bit14 |
799-
trace->type.bit15 | trace->type.bit16 | trace->type.bit17 |
800-
trace->type.bit18 | trace->type.bit19 | trace->type.bit20 |
801-
trace->type.bit21) {
856+
if (trace->overflow)
802857
return;
803-
}
804858

805859
/* NodeLen does not include Opaque State Snapshot length. We need to
806860
* take it into account if the corresponding bit is set (bit 22) and

‎net/ipv6/ioam6_iptunnel.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ static bool ioam6_validate_trace_hdr(struct ioam6_trace_hdr *trace)
7575
u32 fields;
7676

7777
if (!trace->type_be32 || !trace->remlen ||
78-
trace->remlen > IOAM6_TRACE_DATA_SIZE_MAX / 4)
78+
trace->remlen > IOAM6_TRACE_DATA_SIZE_MAX / 4 ||
79+
trace->type.bit12 | trace->type.bit13 | trace->type.bit14 |
80+
trace->type.bit15 | trace->type.bit16 | trace->type.bit17 |
81+
trace->type.bit18 | trace->type.bit19 | trace->type.bit20 |
82+
trace->type.bit21)
7983
return false;
8084

8185
trace->nodelen = 0;

‎tools/testing/selftests/net/ioam6.sh

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -468,10 +468,26 @@ out_bits()
468468
for i in {0..22}
469469
do
470470
ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace \
471-
prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} dev veth0
472-
473-
run_test "out_bit$i" "${desc/<n>/$i}" ioam-node-alpha ioam-node-beta \
474-
db01::2 db01::1 veth0 ${bit2type[$i]} 123
471+
prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} \
472+
dev veth0 &>/dev/null
473+
474+
local cmd_res=$?
475+
local descr="${desc/<n>/$i}"
476+
477+
if [[ $i -ge 12 && $i -le 21 ]]
478+
then
479+
if [ $cmd_res != 0 ]
480+
then
481+
npassed=$((npassed+1))
482+
log_test_passed "$descr"
483+
else
484+
nfailed=$((nfailed+1))
485+
log_test_failed "$descr"
486+
fi
487+
else
488+
run_test "out_bit$i" "$descr" ioam-node-alpha ioam-node-beta \
489+
db01::2 db01::1 veth0 ${bit2type[$i]} 123
490+
fi
475491
done
476492

477493
bit2size[22]=$tmp
@@ -544,7 +560,7 @@ in_bits()
544560
local tmp=${bit2size[22]}
545561
bit2size[22]=$(( $tmp + ${#BETA[9]} + ((4 - (${#BETA[9]} % 4)) % 4) ))
546562

547-
for i in {0..22}
563+
for i in {0..11} {22..22}
548564
do
549565
ip -netns ioam-node-alpha route change db01::/64 encap ioam6 trace \
550566
prealloc type ${bit2type[$i]} ns 123 size ${bit2size[$i]} dev veth0

‎tools/testing/selftests/net/ioam6_parser.c

Lines changed: 60 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,6 @@ enum {
9494
TEST_OUT_BIT9,
9595
TEST_OUT_BIT10,
9696
TEST_OUT_BIT11,
97-
TEST_OUT_BIT12,
98-
TEST_OUT_BIT13,
99-
TEST_OUT_BIT14,
100-
TEST_OUT_BIT15,
101-
TEST_OUT_BIT16,
102-
TEST_OUT_BIT17,
103-
TEST_OUT_BIT18,
104-
TEST_OUT_BIT19,
105-
TEST_OUT_BIT20,
106-
TEST_OUT_BIT21,
10797
TEST_OUT_BIT22,
10898
TEST_OUT_FULL_SUPP_TRACE,
10999

@@ -125,16 +115,6 @@ enum {
125115
TEST_IN_BIT9,
126116
TEST_IN_BIT10,
127117
TEST_IN_BIT11,
128-
TEST_IN_BIT12,
129-
TEST_IN_BIT13,
130-
TEST_IN_BIT14,
131-
TEST_IN_BIT15,
132-
TEST_IN_BIT16,
133-
TEST_IN_BIT17,
134-
TEST_IN_BIT18,
135-
TEST_IN_BIT19,
136-
TEST_IN_BIT20,
137-
TEST_IN_BIT21,
138118
TEST_IN_BIT22,
139119
TEST_IN_FULL_SUPP_TRACE,
140120

@@ -199,30 +179,6 @@ static int check_ioam_header(int tid, struct ioam6_trace_hdr *ioam6h,
199179
ioam6h->nodelen != 2 ||
200180
ioam6h->remlen;
201181

202-
case TEST_OUT_BIT12:
203-
case TEST_IN_BIT12:
204-
case TEST_OUT_BIT13:
205-
case TEST_IN_BIT13:
206-
case TEST_OUT_BIT14:
207-
case TEST_IN_BIT14:
208-
case TEST_OUT_BIT15:
209-
case TEST_IN_BIT15:
210-
case TEST_OUT_BIT16:
211-
case TEST_IN_BIT16:
212-
case TEST_OUT_BIT17:
213-
case TEST_IN_BIT17:
214-
case TEST_OUT_BIT18:
215-
case TEST_IN_BIT18:
216-
case TEST_OUT_BIT19:
217-
case TEST_IN_BIT19:
218-
case TEST_OUT_BIT20:
219-
case TEST_IN_BIT20:
220-
case TEST_OUT_BIT21:
221-
case TEST_IN_BIT21:
222-
return ioam6h->overflow ||
223-
ioam6h->nodelen ||
224-
ioam6h->remlen != 1;
225-
226182
case TEST_OUT_BIT22:
227183
case TEST_IN_BIT22:
228184
return ioam6h->overflow ||
@@ -326,6 +282,66 @@ static int check_ioam6_data(__u8 **p, struct ioam6_trace_hdr *ioam6h,
326282
*p += sizeof(__u32);
327283
}
328284

285+
if (ioam6h->type.bit12) {
286+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
287+
return 1;
288+
*p += sizeof(__u32);
289+
}
290+
291+
if (ioam6h->type.bit13) {
292+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
293+
return 1;
294+
*p += sizeof(__u32);
295+
}
296+
297+
if (ioam6h->type.bit14) {
298+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
299+
return 1;
300+
*p += sizeof(__u32);
301+
}
302+
303+
if (ioam6h->type.bit15) {
304+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
305+
return 1;
306+
*p += sizeof(__u32);
307+
}
308+
309+
if (ioam6h->type.bit16) {
310+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
311+
return 1;
312+
*p += sizeof(__u32);
313+
}
314+
315+
if (ioam6h->type.bit17) {
316+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
317+
return 1;
318+
*p += sizeof(__u32);
319+
}
320+
321+
if (ioam6h->type.bit18) {
322+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
323+
return 1;
324+
*p += sizeof(__u32);
325+
}
326+
327+
if (ioam6h->type.bit19) {
328+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
329+
return 1;
330+
*p += sizeof(__u32);
331+
}
332+
333+
if (ioam6h->type.bit20) {
334+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
335+
return 1;
336+
*p += sizeof(__u32);
337+
}
338+
339+
if (ioam6h->type.bit21) {
340+
if (__be32_to_cpu(*((__u32 *)*p)) != 0xffffffff)
341+
return 1;
342+
*p += sizeof(__u32);
343+
}
344+
329345
if (ioam6h->type.bit22) {
330346
len = cnf.sc_data ? strlen(cnf.sc_data) : 0;
331347
aligned = cnf.sc_data ? __ALIGN_KERNEL(len, 4) : 0;
@@ -455,26 +471,6 @@ static int str2id(const char *tname)
455471
return TEST_OUT_BIT10;
456472
if (!strcmp("out_bit11", tname))
457473
return TEST_OUT_BIT11;
458-
if (!strcmp("out_bit12", tname))
459-
return TEST_OUT_BIT12;
460-
if (!strcmp("out_bit13", tname))
461-
return TEST_OUT_BIT13;
462-
if (!strcmp("out_bit14", tname))
463-
return TEST_OUT_BIT14;
464-
if (!strcmp("out_bit15", tname))
465-
return TEST_OUT_BIT15;
466-
if (!strcmp("out_bit16", tname))
467-
return TEST_OUT_BIT16;
468-
if (!strcmp("out_bit17", tname))
469-
return TEST_OUT_BIT17;
470-
if (!strcmp("out_bit18", tname))
471-
return TEST_OUT_BIT18;
472-
if (!strcmp("out_bit19", tname))
473-
return TEST_OUT_BIT19;
474-
if (!strcmp("out_bit20", tname))
475-
return TEST_OUT_BIT20;
476-
if (!strcmp("out_bit21", tname))
477-
return TEST_OUT_BIT21;
478474
if (!strcmp("out_bit22", tname))
479475
return TEST_OUT_BIT22;
480476
if (!strcmp("out_full_supp_trace", tname))
@@ -509,26 +505,6 @@ static int str2id(const char *tname)
509505
return TEST_IN_BIT10;
510506
if (!strcmp("in_bit11", tname))
511507
return TEST_IN_BIT11;
512-
if (!strcmp("in_bit12", tname))
513-
return TEST_IN_BIT12;
514-
if (!strcmp("in_bit13", tname))
515-
return TEST_IN_BIT13;
516-
if (!strcmp("in_bit14", tname))
517-
return TEST_IN_BIT14;
518-
if (!strcmp("in_bit15", tname))
519-
return TEST_IN_BIT15;
520-
if (!strcmp("in_bit16", tname))
521-
return TEST_IN_BIT16;
522-
if (!strcmp("in_bit17", tname))
523-
return TEST_IN_BIT17;
524-
if (!strcmp("in_bit18", tname))
525-
return TEST_IN_BIT18;
526-
if (!strcmp("in_bit19", tname))
527-
return TEST_IN_BIT19;
528-
if (!strcmp("in_bit20", tname))
529-
return TEST_IN_BIT20;
530-
if (!strcmp("in_bit21", tname))
531-
return TEST_IN_BIT21;
532508
if (!strcmp("in_bit22", tname))
533509
return TEST_IN_BIT22;
534510
if (!strcmp("in_full_supp_trace", tname))
@@ -606,16 +582,6 @@ static int (*func[__TEST_MAX])(int, struct ioam6_trace_hdr *, __u32, __u16) = {
606582
[TEST_OUT_BIT9] = check_ioam_header_and_data,
607583
[TEST_OUT_BIT10] = check_ioam_header_and_data,
608584
[TEST_OUT_BIT11] = check_ioam_header_and_data,
609-
[TEST_OUT_BIT12] = check_ioam_header,
610-
[TEST_OUT_BIT13] = check_ioam_header,
611-
[TEST_OUT_BIT14] = check_ioam_header,
612-
[TEST_OUT_BIT15] = check_ioam_header,
613-
[TEST_OUT_BIT16] = check_ioam_header,
614-
[TEST_OUT_BIT17] = check_ioam_header,
615-
[TEST_OUT_BIT18] = check_ioam_header,
616-
[TEST_OUT_BIT19] = check_ioam_header,
617-
[TEST_OUT_BIT20] = check_ioam_header,
618-
[TEST_OUT_BIT21] = check_ioam_header,
619585
[TEST_OUT_BIT22] = check_ioam_header_and_data,
620586
[TEST_OUT_FULL_SUPP_TRACE] = check_ioam_header_and_data,
621587
[TEST_IN_UNDEF_NS] = check_ioam_header,
@@ -633,16 +599,6 @@ static int (*func[__TEST_MAX])(int, struct ioam6_trace_hdr *, __u32, __u16) = {
633599
[TEST_IN_BIT9] = check_ioam_header_and_data,
634600
[TEST_IN_BIT10] = check_ioam_header_and_data,
635601
[TEST_IN_BIT11] = check_ioam_header_and_data,
636-
[TEST_IN_BIT12] = check_ioam_header,
637-
[TEST_IN_BIT13] = check_ioam_header,
638-
[TEST_IN_BIT14] = check_ioam_header,
639-
[TEST_IN_BIT15] = check_ioam_header,
640-
[TEST_IN_BIT16] = check_ioam_header,
641-
[TEST_IN_BIT17] = check_ioam_header,
642-
[TEST_IN_BIT18] = check_ioam_header,
643-
[TEST_IN_BIT19] = check_ioam_header,
644-
[TEST_IN_BIT20] = check_ioam_header,
645-
[TEST_IN_BIT21] = check_ioam_header,
646602
[TEST_IN_BIT22] = check_ioam_header_and_data,
647603
[TEST_IN_FULL_SUPP_TRACE] = check_ioam_header_and_data,
648604
[TEST_FWD_FULL_SUPP_TRACE] = check_ioam_header_and_data,

0 commit comments

Comments
 (0)