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;
+ }
VLOG_INFO("%s", log_msg);
if (c->key.dl_type == htons(ETH_TYPE_IP)) {
VLOG_INFO("src addr "IP_FMT " dst addr "IP_FMT
..
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://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
