Hi Ben,
I found an issue in the lines below. It looks like that the 'else if' part
is redundant.
+if (dp_packet_size(packet) < packet_size) {
+packet_expand(packet, , packet_size);
+} else if (dp_packet_size(packet) < packet_size){
+dp_packet_delete(packet);
+packet = NULL;
+}
And do you know which commit this patch is based on? I am having problem
to apply this patch and test it out.
Thanks,
Yifeng
On Thu, Jan 18, 2018 at 2:45 PM, Ben Pfaff wrote:
> This makes traffic generated by flow_compose() look slightly more
> realistic. It requires lots of updates to tests, but at least the tests
> themselves should be slightly more realistic too.
>
> At the same time, add --l7 and --l7-len options to ofproto/trace to allow
> users to specify the amount or contents of payloads that they want.
>
> Suggested-by: Brad Cowie
> Signed-off-by: Ben Pfaff
> ---
> NEWS | 3 +-
> lib/flow.c | 88 +++--
> lib/flow.h | 4 +-
> lib/netdev-dummy.c | 15 +-
> ofproto/ofproto-dpif-trace.c | 34 +-
> ofproto/ofproto-dpif.c | 2 +-
> ofproto/ofproto-unixctl.man | 10 +
> ovn/controller/ofctrl.c | 2 +-
> tests/bfd.at | 4 +-
> tests/learn.at | 40 +--
> tests/mcast-snooping.at | 2 +-
> tests/ofproto-dpif.at| 806 --
> -
> tests/pmd.at | 52 +--
> tests/test-ovn.c | 2 +-
> 14 files changed, 620 insertions(+), 444 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index c067b9462f2d..5289bc156dc2 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,7 @@
> Post-v2.9.0
>
> -- Nothing yet.
> +- ovs-vswitchd:
> + * New options --l7 and --l7-len to "ofproto/trace" command.
>
>
> v2.9.0 - xx xxx
> diff --git a/lib/flow.c b/lib/flow.c
> index 04a73fd4ed5a..38ff29c8cd14 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -2692,8 +2692,24 @@ flow_set_mpls_lse(struct flow *flow, int idx,
> ovs_be32 lse)
> flow->mpls_lse[idx] = lse;
> }
>
> +static void
> +flow_compose_l7(struct dp_packet *p, const void *l7, size_t l7_len)
> +{
> +if (l7_len) {
> +if (l7) {
> +dp_packet_put(p, l7, l7_len);
> +} else {
> +uint8_t *payload = dp_packet_put_uninit(p, l7_len);
> +for (size_t i = 0; i < l7_len; i++) {
> +payload[i] = i;
> +}
> +}
> +}
> +}
> +
> static size_t
> -flow_compose_l4(struct dp_packet *p, const struct flow *flow)
> +flow_compose_l4(struct dp_packet *p, const struct flow *flow,
> +const void *l7, size_t l7_len)
> {
> size_t orig_len = dp_packet_size(p);
>
> @@ -2704,19 +2720,31 @@ flow_compose_l4(struct dp_packet *p, const struct
> flow *flow)
> tcp->tcp_src = flow->tp_src;
> tcp->tcp_dst = flow->tp_dst;
> tcp->tcp_ctl = TCP_CTL(ntohs(flow->tcp_flags), 5);
> +if (!(flow->tcp_flags & htons(TCP_SYN | TCP_FIN | TCP_RST))) {
> +flow_compose_l7(p, l7, l7_len);
> +}
> } else if (flow->nw_proto == IPPROTO_UDP) {
> struct udp_header *udp = dp_packet_put_zeros(p, sizeof *udp);
> udp->udp_src = flow->tp_src;
> udp->udp_dst = flow->tp_dst;
> -udp->udp_len = htons(sizeof *udp);
> +udp->udp_len = htons(sizeof *udp + l7_len);
> +flow_compose_l7(p, l7, l7_len);
> } else if (flow->nw_proto == IPPROTO_SCTP) {
> struct sctp_header *sctp = dp_packet_put_zeros(p, sizeof
> *sctp);
> sctp->sctp_src = flow->tp_src;
> sctp->sctp_dst = flow->tp_dst;
> +/* XXX Someone should figure out what L7 data to include. */
> } else if (flow->nw_proto == IPPROTO_ICMP) {
> struct icmp_header *icmp = dp_packet_put_zeros(p, sizeof
> *icmp);
> icmp->icmp_type = ntohs(flow->tp_src);
> icmp->icmp_code = ntohs(flow->tp_dst);
> +if ((icmp->icmp_type == ICMP4_ECHO_REQUEST ||
> + icmp->icmp_type == ICMP4_ECHO_REPLY)
> +&& icmp->icmp_code == 0) {
> +flow_compose_l7(p, l7, l7_len);
> +} else {
> +/* XXX Add inner IP packet for e.g. destination
> unreachable? */
> +}
> } else if (flow->nw_proto == IPPROTO_IGMP) {
> struct igmp_header *igmp = dp_packet_put_zeros(p, sizeof
> *igmp);
> igmp->igmp_type = ntohs(flow->tp_src);
> @@ -2748,6 +2776,12 @@ flow_compose_l4(struct dp_packet *p, const struct
> flow *flow)
> lla_opt->type = ND_OPT_TARGET_LINKADDR;
> lla_opt->mac = flow->arp_tha;
> }
> +} else if (icmp->icmp6_code == 0 &&
> +
