| Message ID | 1587046640-41943-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] conntrack: Add coverage count for l4csum error. | expand |
On Thu, Apr 16, 2020 at 7:17 AM William Tu <u9012063@gmail.com> wrote: > > Add a coverage counter when userspace conntrack receives a packet > with invalid l4 checksum. When using veth for testing, users > often forget to turn off the tx offload on the other side of the > namespace, causing l4 checksum not calculated in packet header, > and at conntrack, return invalid conntrack state. > > Suggested-by: Yi-Hung Wei <yihung.wei@gmail.com> > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/conntrack.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 0cbc8f6d2b25..98a62ce3bae6 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -44,6 +44,7 @@ VLOG_DEFINE_THIS_MODULE(conntrack); > > COVERAGE_DEFINE(conntrack_full); > COVERAGE_DEFINE(conntrack_long_cleanup); > +COVERAGE_DEFINE(conntrack_l4csum_err); > > struct conn_lookup_ctx { > struct conn_key key; > @@ -1661,6 +1662,7 @@ checksum_valid(const struct conn_key *key, const void *data, size_t size, > } else if (key->dl_type == htons(ETH_TYPE_IPV6)) { > return packet_csum_upperlayer6(l3, data, key->nw_proto, size) == 0; > } else { > + COVERAGE_INC(conntrack_l4csum_err); > return false; > } > } > -- Hi William, I think this patch does count the l4 checksum error on tcp, udp and icmpv6. But it does not cover the case of icmp v4. Should we include the checksum error on icmp v4 as well? Thanks, -Yi-Hung
On Thu, Apr 16, 2020 at 10:35 AM Yi-Hung Wei <yihung.wei@gmail.com> wrote: > > On Thu, Apr 16, 2020 at 7:17 AM William Tu <u9012063@gmail.com> wrote: > > > > Add a coverage counter when userspace conntrack receives a packet > > with invalid l4 checksum. When using veth for testing, users > > often forget to turn off the tx offload on the other side of the > > namespace, causing l4 checksum not calculated in packet header, > > and at conntrack, return invalid conntrack state. > > > > Suggested-by: Yi-Hung Wei <yihung.wei@gmail.com> > > Signed-off-by: William Tu <u9012063@gmail.com> > > --- > > lib/conntrack.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/lib/conntrack.c b/lib/conntrack.c > > index 0cbc8f6d2b25..98a62ce3bae6 100644 > > --- a/lib/conntrack.c > > +++ b/lib/conntrack.c > > @@ -44,6 +44,7 @@ VLOG_DEFINE_THIS_MODULE(conntrack); > > > > COVERAGE_DEFINE(conntrack_full); > > COVERAGE_DEFINE(conntrack_long_cleanup); > > +COVERAGE_DEFINE(conntrack_l4csum_err); > > > > struct conn_lookup_ctx { > > struct conn_key key; > > @@ -1661,6 +1662,7 @@ checksum_valid(const struct conn_key *key, const void *data, size_t size, > > } else if (key->dl_type == htons(ETH_TYPE_IPV6)) { > > return packet_csum_upperlayer6(l3, data, key->nw_proto, size) == 0; > > } else { > > + COVERAGE_INC(conntrack_l4csum_err); > > return false; > > } > > } > > -- > > Hi William, > > I think this patch does count the l4 checksum error on tcp, udp and > icmpv6. But it does not cover the case of icmp v4. Should we include > the checksum error on icmp v4 as well? > OK I will add it. William
diff --git a/lib/conntrack.c b/lib/conntrack.c index 0cbc8f6d2b25..98a62ce3bae6 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -44,6 +44,7 @@ VLOG_DEFINE_THIS_MODULE(conntrack); COVERAGE_DEFINE(conntrack_full); COVERAGE_DEFINE(conntrack_long_cleanup); +COVERAGE_DEFINE(conntrack_l4csum_err); struct conn_lookup_ctx { struct conn_key key; @@ -1661,6 +1662,7 @@ checksum_valid(const struct conn_key *key, const void *data, size_t size, } else if (key->dl_type == htons(ETH_TYPE_IPV6)) { return packet_csum_upperlayer6(l3, data, key->nw_proto, size) == 0; } else { + COVERAGE_INC(conntrack_l4csum_err); return false; } }
Add a coverage counter when userspace conntrack receives a packet with invalid l4 checksum. When using veth for testing, users often forget to turn off the tx offload on the other side of the namespace, causing l4 checksum not calculated in packet header, and at conntrack, return invalid conntrack state. Suggested-by: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: William Tu <u9012063@gmail.com> --- lib/conntrack.c | 2 ++ 1 file changed, 2 insertions(+)