On 1/19/26 5:57 PM, Aaron Conole wrote:
> Eelco Chaudron via dev <[email protected]> writes:
>
>> Add the '--len' option to 'ovs-ofctl compose-packet', which
>> allows truncation or extension (with zeros) of the generated
>> packet.
>>
>> Fixes: 90b6e83beb60 ("tests: Fix NSH decap header test for real Ethernet
>> devices.")
>> Signed-off-by: Eelco Chaudron <[email protected]>
>> ---
>> tests/system-traffic.at | 4 ++--
>> utilities/ovs-ofctl.c | 31 ++++++++++++++++++++++++++++---
>> 2 files changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index 8f4fdf8b1..71a8afff2 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -9996,12 +9996,12 @@ dnl Send the NSH packet with TCP SYN payload from
>> p0(at_ns0) interface directed
>> dnl to p1(at_ns1) interface.
>> NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 \
>> "$(ovs-ofctl compose-packet --bare 'NSH_HEADER')" \
>> - "$(ovs-ofctl compose-packet --bare 'TCP_SYN_PKT')"],
>> + "$(ovs-ofctl compose-packet --len 64 --bare 'TCP_SYN_PKT')"],
>> [0], [ignore])
>>
>> dnl Check the expected de-capsulated TCP packet on the egress interface
>> OVS_WAIT_UNTIL([ovs-pcap p1.pcap | grep -q \
>> - "^$(ovs-ofctl compose-packet --bare 'TCP_SYN_PKT')0*\$"])
>> + "^$(ovs-ofctl compose-packet --len 64 --bare 'TCP_SYN_PKT')\$"])
>>
>> OVS_TRAFFIC_VSWITCHD_STOP
>> AT_CLEANUP
>> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
>> index ba3458e55..d949583c6 100644
>> --- a/utilities/ovs-ofctl.c
>> +++ b/utilities/ovs-ofctl.c
>> @@ -160,6 +160,9 @@ static int print_bare = 0;
>> /* -bad-csum: Makes "compose-packet" generate an invalid checksum. */
>> static int bad_csum = 0;
>>
>> +/* --len: Makes "compose-packet" truncate or extend the payload. */
>> +static unsigned int opt_len = 0;
>> +
>> /* --raw: Makes "ofp-print" read binary data from stdin. */
>> static int raw = 0;
>>
>> @@ -223,6 +226,7 @@ parse_options(int argc, char *argv[])
>> OPT_COLOR,
>> OPT_MAY_CREATE,
>> OPT_READ_ONLY,
>> + OPT_LEN,
>> DAEMON_OPTION_ENUMS,
>> OFP_VERSION_OPTION_ENUMS,
>> VLOG_OPTION_ENUMS,
>> @@ -253,6 +257,7 @@ parse_options(int argc, char *argv[])
>> {"bad-csum", no_argument, &bad_csum, 1},
>> {"raw", no_argument, &raw, 1},
>> {"read-only", no_argument, NULL, OPT_READ_ONLY},
>> + {"len", required_argument, NULL, OPT_LEN},
>> DAEMON_LONG_OPTIONS,
>> OFP_VERSION_LONG_OPTIONS,
>> VLOG_LONG_OPTIONS,
>> @@ -382,6 +387,12 @@ parse_options(int argc, char *argv[])
>> may_create = true;
>> break;
>>
>> + case OPT_LEN:
>> + if (!str_to_uint(optarg, 10, &opt_len)) {
>> + ovs_fatal(0, "value %s on --len is invalid", optarg);
>> + }
>> + break;
>> +
>> DAEMON_OPTION_HANDLERS
>> OFP_VERSION_OPTION_HANDLERS
>> VLOG_OPTION_HANDLERS
>> @@ -4934,9 +4945,10 @@ ofctl_parse_key_value(struct ovs_cmdl_context *ctx)
>> }
>> }
>>
>> -/* "compose-packet [--pcap|--bare] [--bad-csum] FLOW [L7]": Converts the
>> - * OpenFlow flow specification FLOW to a packet with flow_compose() and
>> prints
>> - * the hex bytes of the packet, with offsets, to stdout.
>> +/* "compose-packet [--pcap|--bare] [--bad-csum] [--len <length>] FLOW [L7]":
>> + * Converts the OpenFlow flow specification FLOW to a packet with
>> + * flow_compose() and prints the hex bytes of the packet, with offsets, to
>> + * stdout.
>> *
>> * With --pcap, prints the packet in pcap format, so that you can do
>> something
>> * like "ovs-ofctl --pcap compose-packet udp | tcpdump -vvvv -r-" to use
>> @@ -4949,6 +4961,9 @@ ofctl_parse_key_value(struct ovs_cmdl_context *ctx)
>> *
>> * With --bad-csum, produces a packet with an invalid IP checksum. (For
>> IPv4.)
>> *
>> + * With --len, produces a packet that is either truncated or extended (with
>> + * zeros) to the given length.
>> + *
>> * Regardless of the mode, the command also verifies that the flow extracted
>> * from that packet matches the original FLOW.
>> *
>> @@ -4991,6 +5006,16 @@ ofctl_compose_packet(struct ovs_cmdl_context *ctx)
>> flow_compose(&p, &flow1, l7, l7_len, bad_csum);
>> free(l7);
>>
>> + if (opt_len) {
>> + size_t p_len = dp_packet_size(&p);
>> +
>> + if (opt_len > p_len) {
>> + dp_packet_put_zeros(&p, opt_len - p_len);
>> + } else {
>> + dp_packet_set_size(&p, opt_len);
>> + }
>> + }
>> +
>
> I think we should be using packet_expand() instead of the direct
> dp_packet_put_zeros call - the reason is to ensure that checksums are
> properly regenerated. WDYT?
+1 from me on this one. Main part is length fields though. Without
updating the length in the header, this is just padding.
>
>> if (print_pcap) {
>> struct pcap_file *p_file = ovs_pcap_stdout();
>> ovs_pcap_write_header(p_file);
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
