On 9/21/17, 4:01 AM, "ovs-dev-boun...@openvswitch.org on behalf of Fischetti, Antonio" <ovs-dev-boun...@openvswitch.org on behalf of antonio.fische...@intel.com> wrote:
Looks a good improvement to report on an intentional exploit, or other issues. LGTM, just one comment. Now this new print fn is called just from create_un_nat_conn but in the future it could potentially be called from any other CT function. As the function call could affect the performance, especially for the memcpy when dl_type != ETH_TYPE_IP, I was wondering if print_conn_info could be limited to work over a certain info level. Could something like the following help on this? void print_conn_info(struct conn *c, char *log_msg) { + if (!VLOG_IS_DBG_ENABLED()) { + return; + } Good idea, thanks. I think we go one step further and pass in the log level and do the checking you mentioned against the requested log level. BTW: I ended up improving the V6 address print handling to the following: ========================= char ip6_s0[INET6_ADDRSTRLEN]; inet_ntop(AF_INET6, &c->key.src.addr.ipv6, ip6_s0, sizeof ip6_s0); char ip6_s1[INET6_ADDRSTRLEN]; inet_ntop(AF_INET6, &c->key.dst.addr.ipv6, ip6_s1, sizeof ip6_s1); char ip6_s2[INET6_ADDRSTRLEN]; inet_ntop(AF_INET6, &c->rev_key.src.addr.ipv6, ip6_s2, sizeof ip6_s2); char ip6_s3[INET6_ADDRSTRLEN]; inet_ntop(AF_INET6, &c->rev_key.dst.addr.ipv6, ip6_s3, sizeof ip6_s3); VLOG_INFO("src addr %s; dst addr %s; rev src addr %s; rev dst addr %s", ip6_s0, ip6_s1, ip6_s2, ip6_s3); ========================= Thanks Darrell .. Acked-by: Antonio Fischetti <antonio.fische...@intel.com> > -----Original Message----- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-boun...@openvswitch.org] > On Behalf Of Darrell Ball > Sent: Thursday, September 21, 2017 8:12 AM > To: dlu...@gmail.com; d...@openvswitch.org > Subject: [ovs-dev] [patch v2 4/5] conntrack: Add function print_conn_info(). > > A new debug function is added and used in a > subsequent patch. > > Signed-off-by: Darrell Ball <dlu...@gmail.com> > --- > lib/conntrack.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 2eca38d..8deeec9 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -67,6 +67,8 @@ enum ct_alg_mode { > CT_TFTP_MODE, > }; > > +void print_conn_info(struct conn *c, char *log_msg); > + > static bool conn_key_extract(struct conntrack *, struct dp_packet *, > ovs_be16 dl_type, struct conn_lookup_ctx *, > uint16_t zone); > @@ -223,6 +225,57 @@ conn_key_cmp(const struct conn_key *key1, const struct > conn_key *key2) > return 1; > } > > +void > +print_conn_info(struct conn *c, char *log_msg) > +{ > + VLOG_INFO("%s", log_msg); > + if (c->key.dl_type == htons(ETH_TYPE_IP)) { > + VLOG_INFO("src addr "IP_FMT " dst addr "IP_FMT > + " rev src addr "IP_FMT " rev dst addr "IP_FMT, > + IP_ARGS(c->key.src.addr.ipv4_aligned), > + IP_ARGS(c->key.dst.addr.ipv4_aligned), > + IP_ARGS(c->rev_key.src.addr.ipv4_aligned), > + IP_ARGS(c->rev_key.dst.addr.ipv4_aligned)); > + } else { > + ovs_be32 a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13; > + ovs_be32 a14, a15; > + memcpy(&a0, &c->key.src.addr.ipv6_aligned.s6_addr[0], sizeof a0); > + memcpy(&a1, &c->key.src.addr.ipv6_aligned.s6_addr[4], sizeof a1); > + memcpy(&a2, &c->key.src.addr.ipv6_aligned.s6_addr[8], sizeof a2); > + memcpy(&a3, &c->key.src.addr.ipv6_aligned.s6_addr[12], sizeof a3); > + memcpy(&a4, &c->key.dst.addr.ipv6_aligned.s6_addr[0], sizeof a4); > + memcpy(&a5, &c->key.dst.addr.ipv6_aligned.s6_addr[4], sizeof a5); > + memcpy(&a6, &c->key.dst.addr.ipv6_aligned.s6_addr[8], sizeof a6); > + memcpy(&a7, &c->key.dst.addr.ipv6_aligned.s6_addr[12], sizeof a7); > + memcpy(&a8, &c->rev_key.src.addr.ipv6_aligned.s6_addr[0], > + sizeof a8); > + memcpy(&a9, &c->rev_key.src.addr.ipv6_aligned.s6_addr[4], > + sizeof a9); > + memcpy(&a10, &c->rev_key.src.addr.ipv6_aligned.s6_addr[8], > + sizeof a10); > + memcpy(&a11, &c->rev_key.src.addr.ipv6_aligned.s6_addr[12], > + sizeof a11); > + memcpy(&a12, &c->rev_key.dst.addr.ipv6_aligned.s6_addr[0], > + sizeof a12); > + memcpy(&a13, &c->rev_key.dst.addr.ipv6_aligned.s6_addr[4], > + sizeof a13); > + memcpy(&a14, &c->rev_key.dst.addr.ipv6_aligned.s6_addr[8], > + sizeof a14); > + memcpy(&a15, &c->rev_key.dst.addr.ipv6_aligned.s6_addr[12], > + sizeof a15); > + > + VLOG_INFO("src addr 0x%08x:%08x:%08x:%08x; " > + "dst addr 0x%08x:%08x:%08x:%08x; " > + "rev src addr 0x%08x:%08x:%08x:%08x; " > + "rev dst addr 0x%08x:%08x:%08x:%08x", ntohl(a0), ntohl(a1), > + ntohl(a2), ntohl(a3), ntohl(a4), ntohl(a5), ntohl(a6), > + ntohl(a7), ntohl(a8), ntohl(a9), ntohl(a10), ntohl(a11), > + ntohl(a12), ntohl(a13), ntohl(a14), ntohl(a15)); > + } > + VLOG_INFO("src/dst ports %d/%d rev src/dst ports %d/%d", c->key.src.port, > + c->key.dst.port, c->rev_key.src.port, c->rev_key.dst.port); > +} > + > /* Initializes the connection tracker 'ct'. The caller is responsible for > * calling 'conntrack_destroy()', when the instance is not needed anymore */ > void > -- > 1.9.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=KJTV1pFisuVRJMYQggKXHW2nx5C-1FVotX7ZrszEYlM&s=QpT44FCD4Eg8jtTYFEn6ySJOOfO1uwKYKW7DEEczf_c&e= _______________________________________________ dev mailing list d...@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=KJTV1pFisuVRJMYQggKXHW2nx5C-1FVotX7ZrszEYlM&s=QpT44FCD4Eg8jtTYFEn6ySJOOfO1uwKYKW7DEEczf_c&e= _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev