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.)

And yes, I see that gcc is warning about this line in CI. So this line may need adjustment for this reason.

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.
+        } 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

Reply via email to