* Re: [PATCH] app/testpmd: fix auto completion for indirect list action
2024-03-20 10:08 ` Ferruh Yigit
@ 2024-03-20 20:25 ` Patrick Robb
2024-03-23 10:09 ` Gregory Etelson
1 sibling, 0 replies; 5+ messages in thread
From: Patrick Robb @ 2024-03-20 20:25 UTC (permalink / raw)
To: Ferruh Yigit
Cc: Gregory Etelson, Shani Peretz, dev, Maayan Kashani, Ori Kam,
Aman Singh, Yuying Zhang, ci, Honnappa Nagarahalli
On Wed, Mar 20, 2024 at 6:08 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 3/20/2024 6:06 AM, Gregory Etelson wrote:
> > Hello Ferruh,
> >
> >>BUT overall how can we catch issues like this in the feature, we don't
> >>have a good way to test testpmd flow commands.
> >>@Ori, @Gregory, do you have any idea?
> >>cc'ed CI mail list too.
> >
> > We have a tool for unit tests based on the testpmd.
> > The tool details are here:
> > https://drive.google.com/drive/folders/1cHrPwx4fUJ6ibUCtHd4kNKsrmmvQvvOj?usp=drive_link <https://drive.google.com/drive/folders/1cHrPwx4fUJ6ibUCtHd4kNKsrmmvQvvOj?usp=drive_link>.
> > There's also a short description here:
> > https://inbox.dpdk.org/ci/2a287ee7-cda4-f2ab-a4e6-a47021f8573f@nvidia.com/ <https://inbox.dpdk.org/ci/2a287ee7-cda4-f2ab-a4e6-a47021f8573f@nvidia.com/>
> >
> > Consider an option when a code patch is accompanied with a short test
> > script that validates that patch functionality.
> > DPDK CI can run the script to verify that the patch functions correctly.
A similar idea has been proposed for DTS. I believe the plan is at the
end of 2024 techboard is going to discuss adding this as a requirement
for patches affecting certain components in DPDK. So a change related
to the new feature must come with an accompanied /dts testsuite
addition at the same time.
> >
> >
>
> Thanks Gregory, I missed this proposal, we need something to verify flow
> APIs, so +1 to the effort.
> What is the status of incorporating this feature into dts?
The DTS 24.07 roadmap does not include testsuites verifying flow APIs,
but if work proceeds at a good pace and there is space for it, we can
look at writing something for 24.07. Otherwise it falls to 24.11 or
25.03. Sorry, I know that's pretty far out.
>
>
> But I guess it won't catch this issue, as it uses full flow commands.
> This issue is related to the testpmd command parsing code. I wonder if
> we can find a way to verify testpmd parsing code?
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] app/testpmd: fix auto completion for indirect list action
2024-03-20 10:08 ` Ferruh Yigit
2024-03-20 20:25 ` Patrick Robb
@ 2024-03-23 10:09 ` Gregory Etelson
1 sibling, 0 replies; 5+ messages in thread
From: Gregory Etelson @ 2024-03-23 10:09 UTC (permalink / raw)
To: Ferruh Yigit, Shani Peretz, dev
Cc: Maayan Kashani, Ori Kam, Aman Singh, Yuying Zhang, ci,
Honnappa Nagarahalli, Patrick Robb, Asaf Penso
[-- Attachment #1: Type: text/plain, Size: 5884 bytes --]
Hello Ferruh,
> But I guess it won't catch this issue, as it uses full flow commands.
> This issue is related to the testpmd command parsing code. I wonder if
> we can find a way to verify testpmd parsing code?
What if flow command validation program breaks a tested command into tokens and then pass each token followed by the TAB key to the controlled testpmd process ?
There are 3 possible outputs for the token + TAB sequence:
1. a valid token will trigger meaningful testpmd help ouput that can be matched.
2. invalid token output will fail [1].
3.
invalid token will crash the controlled testpmd process.
I've compiled a small programm to test that idea.
Please check it out.
Regards,
Gregory.
commit e4a27c2c2892cfd408b473b18192b30927e4281c
Author: Gregory Etelson <getelson@nvidia.com>
Date: Sat Mar 23 11:38:44 2024 +0200
validate testpmd flow command
diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 0000000..3498868
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,12 @@
+[package]
+name = "testpmd-validate"
+version = "0.1.0"
+edition = "2021"
+rust-version = "1.76.0"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+ssh2 = { version = "0.9.4", features = ["vendored-openssl"] }
+time = "0.3.34"
+regex = "1.10.4"
diff --git a/src/main.rs b/src/main.rs
new file mode 100644
index 0000000..90dfdac
--- /dev/null
+++ b/src/main.rs
@@ -0,0 +1,128 @@
+use std::io::{Read, Write};
+use std::{io, thread, time};
+use std::path::Path;
+use std::ops::Add;
+use std::time::{SystemTime};
+use ssh2::{Channel, Error, ExtendedData, Session};
+use regex::{RegexSet};
+
+fn continue_after_error(err:&io::Error) -> bool {
+ match err.kind() {
+ io::ErrorKind::WouldBlock => return true,
+ _ => return false,
+ }
+}
+
+fn wrap_ssh<F, T>(mut func:F) -> T
+ where F: FnMut() -> Result<T, Error> {
+ loop {
+ match func() {
+ Ok(val) => { return val },
+ Err(ssh_err) => {
+ let io_err = io::Error::from(ssh_err);
+ if !continue_after_error(&io_err) {
+ panic!("ssh2 error: {:?}", io_err);
+ }
+ }
+ }
+ }
+}
+
+fn rsh_write(channel:&mut Channel, buf:&str) {
+ match channel.write(buf.as_bytes()) {
+ Ok(_) => (),
+ Err(e) => panic!("write failure: {:?}", e)
+ }
+}
+
+const RSH_READ_MAX_DURATION:time::Duration = time::Duration::from_millis(350);
+const RSH_READ_IDLE_TIMEOUT:time::Duration = time::Duration::from_millis(10);
+fn rsh_read(channel:&mut Channel) -> String {
+ let mut end = SystemTime::now().add(RSH_READ_MAX_DURATION);
+ let mut output = String::new();
+ loop {
+ let mut buffer: [u8; 1024] = [0; 1024];
+ match channel.read(&mut buffer) {
+ Ok(0) => { return output }
+ Ok(_) => {
+ let aux = String::from_utf8(buffer.into()).unwrap();
+ // println!("{aux}");
+ output.push_str(&aux);
+ end = SystemTime::now().add(RSH_READ_MAX_DURATION);
+ }
+ Err(err) => {
+ if !continue_after_error(&err) { panic!("read error {:?}", err) }
+ if SystemTime::now().gt(&end) { break; }
+ thread::sleep(RSH_READ_IDLE_TIMEOUT);
+ }
+ }
+ }
+ output
+}
+
+const TESTPMD_PATH:&str = "/tmp/build/app";
+const TESTPMD_CMD:&str = "dpdk-testpmd -a 0000:01:00.0 -- -i";
+
+const TOKEN_PATTERN:[&str; 2] = [
+ r#"(?m)\[.*\]:"#,
+ r#"(?m)\[RETURN]"#,
+];
+
+const CTRL_C:[u8;1] = [0x3];
+
+
+fn rsh_connect(hostname:&str) -> Channel {
+ let stream = std::net::TcpStream::connect(format!("{}:22", hostname)).unwrap();
+ stream.set_nonblocking(true).unwrap();
+ let mut session = Session::new().unwrap();
+ let private_key = Path::new("/home/garik/.ssh/id_rsa");
+ session.set_blocking(false);
+ session.set_tcp_stream(stream);
+ wrap_ssh(|| session.handshake());
+ wrap_ssh(|| session.userauth_pubkey_file("root", None, &private_key, None));
+ let mut ch = wrap_ssh(|| session.channel_session());
+ wrap_ssh(|| ch.handle_extended_data(ExtendedData::Merge));
+ wrap_ssh(|| ch.request_pty("vt100", None, None));
+ ch
+}
+
+fn validate_flow_command(command:&str, ch:&mut Channel) {
+ let rset = RegexSet::new(TOKEN_PATTERN).unwrap();
+ let delay = time::Duration::from_millis(300);
+
+ println!("validate: {command}");
+ for token in command.split_whitespace() {
+ let candid = format!("{token} \t");
+ rsh_write(ch, &candid);
+ print!("validate token \'{token}\' ");
+ thread::sleep(delay);
+ let output = rsh_read(ch);
+ if rset.is_match(&output) {
+ println!(" ok");
+ } else {
+ println!("failed");
+ println!(">>>>>>>>>>>>>>>>>>");
+ println!("{output}");
+ println!("<<<<<<<<<<<<<<<<<<");
+ break;
+ }
+ }
+ let _ = ch.write(&CTRL_C);
+}
+
+fn main() {
+ let mut ch = rsh_connect("10.237.157.85");
+ let command = format!("{}/{}", TESTPMD_PATH, TESTPMD_CMD);
+ wrap_ssh(|| ch.exec(&command));
+ thread::sleep(time::Duration::from_secs(1));
+ println!("{}", rsh_read(&mut ch));
+
+ let good_flow = "flow create 0 ingress group 0 priority 0 pattern eth / end actions drop / end";
+ let bad_flow = "flow create 0 ingress group 0 priority 0 pattern eth/ end actions drop / end";
+
+ validate_flow_command(good_flow, &mut ch);
+ validate_flow_command(bad_flow, &mut ch);
+
+ wrap_ssh(|| ch.close());
+ println!("EOF {:?}", ch.exit_status());
+}
[-- Attachment #2: Type: text/html, Size: 39583 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread