On Fri, Oct 20, 2023 at 6:02 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 10/20/23 15:15, Ihar Hrachyshka wrote: > > On Thu, Oct 19, 2023 at 6:29 PM Ilya Maximets <i.maxim...@ovn.org > <mailto:i.maxim...@ovn.org>> wrote: > > > > On 10/19/23 16:11, Ihar Hrachyshka wrote: > > > The command reads a flow string and an optional additional payload > on > > > stdin and produces a hex-string representation of the corresponding > > > frame on stdout. It may receive more than a single input line. > > > > > > The command may be useful in tests, where we may need to produce > hex > > > frame payloads to compare observed frames against. > > > > > > As an example of the tool use, a single test case is converted to > it. > > > The test uses both normal and --bad-csum behavior of the tool, > > > confirming it works as advertised. > > > > > > Signed-off-by: Ihar Hrachyshka <ihrac...@redhat.com <mailto: > ihrac...@redhat.com>> > > > --- > > > v1: - initial version. > > > v2: - convert from appctl to ovstest. > > > - add --bad-csum support. > > > - drop separate test case and man for the tool. > > > v3: - fix build warnings on restricted ovs_be16 ++. > > > --- > > > lib/flow.c | 16 +++++- > > > lib/flow.h | 2 +- > > > lib/netdev-dummy.c | 4 +- > > > ofproto/ofproto-dpif-trace.c | 2 +- > > > ofproto/ofproto-dpif.c | 4 +- > > > tests/dpif-netdev.at <http://dpif-netdev.at> | 45 > ++++++++-------- > > > tests/test-odp.c | 99 > +++++++++++++++++++++++++++++++++++- > > > utilities/ovs-ofctl.c | 2 +- > > > 8 files changed, 140 insertions(+), 34 deletions(-) > > <snip> > > > > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > > > index 24d0941cf..31da8c2d4 100644 > > > --- a/utilities/ovs-ofctl.c > > > +++ b/utilities/ovs-ofctl.c > > > @@ -4989,7 +4989,7 @@ ofctl_compose_packet(struct ovs_cmdl_context > *ctx) > > > > Hmm. This is interesting. Looks like we have already a commnd with > mostly > > the same behavior, except that it is using OpenFlow format insted of > datapath > > flow format. Can we just use/extend ovs-ofctl compose-packet? > > > > It supports pcap output, so we can use ovs-pcap tool to get the hex > bytes > > in a way they can be fed into netdev-dummy/receive. (We may want to > extend > > ovs-pcap to allow reading from stdin to make that processeasier.) > > > > Instead of > > eth(src=8a:bf:7e:2f:05:84,dst=0a:8f:39:4f:e0:73),eth_type(0x0800),\ > > > ipv4(src=192.168.123.2,dst=192.168.123.1,proto=6,tos=0,ttl=64,frag=no),\ > > tcp(src=54392,dst=5201),tcp_flags(ack)" > > > > We'll just need to write OpenFlow: > > > eth_src=8a:bf:7e:2f:05:84,eth_dst=0a:8f:39:4f:e0:73,dl_type=0x0800,\ > > > nw_src=192.168.123.2,nw_dst=192.168.123.1,nw_proto=6,nw_ttl=64,nw_frag=no,\ > > tp_src=8,tp_dst=80,tcp_flags=ack > > or > > tcp,eth_src=8a:bf:7e:2f:05:84,eth_dst=0a:8f:39:4f:e0:73,\ > > nw_src=192.168.123.2,nw_dst=192.168.123.1,nw_ttl=64,nw_frag=no,\ > > tp_src=8,tp_dst=80,tcp_flags=ack > > > > What do you think? Is there a specific reson for using odp format > over OF? > > > > > > Lol! And a nice catch. > > > > I think it's fair to say that we don't need another tool for the same. > > > > Though compose-packet will need some adjustment to fit the use case, > namely: > > - It currently generates “64 random byte payload” - we may want to make > it “0" or “hardcoded pattern” for test purposes. > > It supports passing L7 payload as a second argument, AFAICT. > Yes; the difference is when the payload is NOT passed, then it generates a random 64 byte payload. It may be fine I guess, just a difference in behavior. I am only concerned that perhaps it may be harder to match against a random payload than against a missing or hardcoded one in some scenarios. (It may also be impossible to generate a "bare" IP packet with an empty payload - which is a valid IP packet.) > > > - We would have to introduce a --bad-csum for the command (and any other > features we'd like in the future). > > Seems fine to me. This one is primarily a testing command, so should > be no problem extending it. > > > - Perhaps also stdin pipelining to simplify the integration in test > cases, as you suggested. > > Yeah, at least ovs-pcap could really use ability to read from stdin. > > > > > If this is the path you'd like to see taken here, we can definitely > switch to it. (I of course hope that would be the last switch of gears.) > > Can't guarantee anything. :D > > > > > Please confirm this is indeed how you see compose-packet evolve, and I > will send another iteration, now for compose-packet. > > At this point in time, I think it's a reasonable approach. > > There is also a parse-packet command. It's only used in fuzzing today. > We could teach it to parse from hex/pcap if needed, I guess. > > Best regards, Ilya Maximets. > > Ack. I'll move to compose-packet then. I will also explore other tools in the suite, it looks like there's some space for self-education. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
