On Thu, Oct 19, 2023 at 09:00:39AM -0400, Ihar Hrachyshka wrote: > On 10/19/23 7:14 AM, Simon Horman wrote: > > On Thu, Oct 19, 2023 at 02:57:26AM +0000, 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> > > ... > > > > > @@ -3318,7 +3320,7 @@ packet_expand(struct dp_packet *p, const struct > > > flow *flow, size_t size) > > > * from 'l7'. */ > > > void > > > flow_compose(struct dp_packet *p, const struct flow *flow, > > > - const void *l7, size_t l7_len) > > > + const void *l7, size_t l7_len, bool bad_csum) > > > { > > > /* Add code to this function (or its callees) for emitting new > > > fields or > > > * protocols. (This isn't essential, so it can be skipped for > > > initial > > > @@ -3370,7 +3372,12 @@ flow_compose(struct dp_packet *p, const struct > > > flow *flow, > > > /* Checksum has already been zeroed by put_zeros call. */ > > > ip->ip_csum = csum(ip, sizeof *ip); > > > > > > - dp_packet_ol_set_ip_csum_good(p); > > > + if (bad_csum) { > > > + ip->ip_csum++; > > Hi Ihar, > > > > I am curious to know why '++'. > > I'm producing a checksum that is invalid. Since Internet checksum is "16-bit > ones' complement of the ones' complement sum", I assume that flipping any > bit in it will result in an invalid result. Is it not the case? (++ could be > anything else that guarantees a bit flip in the field. Nothing special about > ++ itself.)
Yes, that is correct AFAIK. > And yes, I see that gcc is warning about this line in CI. So this line may > need adjustment for this reason. Thanks > > I am also wondering if we could re-cast this as 'skip-csum-verify' or > > similar, rather than 'bad-csum'. After all, as it's not verified it might > > actually be correct. > I am struggling to see how this could be "correct" after ++ on the final > (valid) checksum. It may be that I miss something basic about how checksums > work though, let me know. I guess my question is really this: what is the goal and use-case for this option? > > > + } else { > > > + dp_packet_ol_set_ip_csum_good(p); > > > + } > > > + > > > pseudo_hdr_csum = packet_csum_pseudoheader(ip); > > > flow_compose_l4_csum(p, flow, pseudo_hdr_csum); > > > } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { > > ... > > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev