On 8/4/22 18:07, Paolo Valerio wrote:
> This patch avoids to show flags_orig/flags_reply key if they have no value.
> E.g., the following:
>
> NEW tcp,orig=([...]),reply=([...]),id=1800618864,
> status=CONFIRMED|SRC_NAT_DONE|DST_NAT_DONE,timeout=120,
> protoinfo=(state_orig=SYN_SENT,state_reply=SYN_SENT,wscale_orig=7,
> wscale_reply=0,flags_orig=WINDOW_SCALE|SACK_PERM,flags_reply=)
>
> becomes:
>
> NEW tcp,orig=([...]),reply=([...]),id=1800618864,
> status=CONFIRMED|SRC_NAT_DONE|DST_NAT_DONE,timeout=120,
> protoinfo=(state_orig=SYN_SENT,state_reply=SYN_SENT,wscale_orig=7,
> wscale_reply=0,flags_orig=WINDOW_SCALE|SACK_PERM)
>
> Signed-off-by: Paolo Valerio <[email protected]>
> ---
> lib/ct-dpif.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
> index cfc2315e3..f1a375523 100644
> --- a/lib/ct-dpif.c
> +++ b/lib/ct-dpif.c
> @@ -512,10 +512,16 @@ ct_dpif_format_protoinfo_tcp_verbose(struct ds *ds,
> protoinfo->tcp.wscale_orig,
> protoinfo->tcp.wscale_reply);
> }
> - ct_dpif_format_flags(ds, ",flags_orig=", protoinfo->tcp.flags_orig,
> - tcp_flags);
> - ct_dpif_format_flags(ds, ",flags_reply=", protoinfo->tcp.flags_reply,
> - tcp_flags);
> +
> + if (protoinfo->tcp.flags_orig) {
> + ct_dpif_format_flags(ds, ",flags_orig=", protoinfo->tcp.flags_orig,
> + tcp_flags);
> + }
> +
> + if (protoinfo->tcp.flags_reply) {
> + ct_dpif_format_flags(ds, ",flags_reply=", protoinfo->tcp.flags_reply,
> + tcp_flags);
> + }
Hmm. I'm trying to understand why ct_dpif_format_flags() exists at all.
Shouldn't this be just:
format_flags_masked(ds, "flags_orig", packet_tcp_flag_to_string,
protoinfo->tcp.flags_orig, TCP_FLAGS(OVS_BE16_MAX),
TCP_FLAGS(OVS_BE16_MAX));
?
This will change the appearance of the flags, so maybe tcp_flags[] array
should be replaced with a simple conversion function.
In any case the convention seems to be to print a '0' if the field is empty.
See the implementation of format_flags() function. And that seems to make
sense in the verbose output.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev