On Sat, Apr 29, 2017 at 7:01 PM, Daniele Di Proietto <diproiet...@ovn.org>
wrote:
> 2017-03-24 2:15 GMT-07:00 Darrell Ball <dlu...@gmail.com>:
> > This patch adds orig tuple checking and context
> > recovery; NAT interactions are factored in.
> > Orig tuple support exists to better handle policy
> > changes.
> >
> > Signed-off-by: Darrell Ball <dlu...@gmail.com>
> > ---
> > lib/conntrack.c | 69 ++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++
> > 1 file changed, 69 insertions(+)
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index ee22280..5524495 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -679,6 +679,72 @@ handle_nat(struct dp_packet *pkt, struct conn *conn,
> > }
> > }
> >
> > +static bool
> > +check_orig_tuple(struct conntrack *ct, struct dp_packet *pkt,
> > + struct conn_lookup_ctx *ctx_in, long long now,
> > + unsigned *bucket, struct conn **conn,
> > + const struct nat_action_info_t *nat_action_info)
> > + OVS_REQUIRES(ct->buckets[*bucket].lock)
> > +{
> > + if ((ctx_in->key.dl_type == htons(ETH_TYPE_IP) &&
> > + !pkt->md.ct_orig_tuple.ipv4.ipv4_proto) ||
> > + (ctx_in->key.dl_type == htons(ETH_TYPE_IPV6) &&
> > + !pkt->md.ct_orig_tuple.ipv6.ipv6_proto) ||
> > + !(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
> > + nat_action_info){
> > + return false;
> > + }
> > +
> > + ct_lock_unlock(&ct->buckets[*bucket].lock);
> > + struct conn_lookup_ctx ctx;
> > + memset(&ctx, 0 , sizeof ctx);
> > + ctx.conn = NULL;
> > +
> > + if (ctx_in->key.dl_type == htons(ETH_TYPE_IP)) {
> > +
> > + ctx.key.src.addr.ipv4_aligned = pkt->md.ct_orig_tuple.ipv4.
> ipv4_src;
> > + ctx.key.dst.addr.ipv4_aligned = pkt->md.ct_orig_tuple.ipv4.
> ipv4_dst;
> > +
> > + if (ctx_in->key.nw_proto == IPPROTO_ICMP) {
> > + ctx.key.src.icmp_id = ctx_in->key.src.icmp_id;
> > + ctx.key.dst.icmp_id = ctx_in->key.dst.icmp_id;
> > + uint16_t src_port = ntohs(pkt->md.ct_orig_tuple.
> ipv4.src_port);
> > + ctx.key.src.icmp_type = (uint8_t) src_port;
> > + ctx.key.dst.icmp_type = reverse_icmp_type(ctx.key.src.
> icmp_type);
> > + } else {
> > + ctx.key.src.port = pkt->md.ct_orig_tuple.ipv4.src_port;
> > + ctx.key.dst.port = pkt->md.ct_orig_tuple.ipv4.dst_port;
> > + }
> > + ctx.key.nw_proto = pkt->md.ct_orig_tuple.ipv4.ipv4_proto;
> > + } else {
> > + ctx.key.src.addr.ipv6_aligned = pkt->md.ct_orig_tuple.ipv6.
> ipv6_src;
> > + ctx.key.dst.addr.ipv6_aligned = pkt->md.ct_orig_tuple.ipv6.
> ipv6_dst;
> > +
> > + if (ctx_in->key.nw_proto == IPPROTO_ICMPV6) {
> > + ctx.key.src.icmp_id = ctx_in->key.src.icmp_id;
> > + ctx.key.dst.icmp_id = ctx_in->key.dst.icmp_id;
> > + uint16_t src_port = ntohs(pkt->md.ct_orig_tuple.
> ipv6.src_port);
> > + ctx.key.src.icmp_type = (uint8_t) src_port;
> > + ctx.key.dst.icmp_type = reverse_icmp6_type(ctx.key.
> src.icmp_type);
> > + } else {
> > + ctx.key.src.port = pkt->md.ct_orig_tuple.ipv6.src_port;
> > + ctx.key.dst.port = pkt->md.ct_orig_tuple.ipv6.dst_port;
> > + }
> > + ctx.key.nw_proto = pkt->md.ct_orig_tuple.ipv6.ipv6_proto;
> > + }
> > +
> > + ctx.key.dl_type = ctx_in->key.dl_type;
> > + ctx.key.zone = pkt->md.ct_zone;
> > +
> > + ctx.hash = conn_key_hash(&ctx.key, ct->hash_basis);
> > + *bucket = hash_to_bucket(ctx.hash);
> > + ct_lock_lock(&ct->buckets[*bucket].lock);
> > + conn_key_lookup(&ct->buckets[*bucket], &ctx, now);
> > + *conn = ctx.conn;
> > +
> > + return *conn ? true : false;
> > +}
> > +
> > static void
> > process_one(struct conntrack *ct, struct dp_packet *pkt,
> > struct conn_lookup_ctx *ctx, uint16_t zone,
> > @@ -735,6 +801,9 @@ process_one(struct conntrack *ct, struct dp_packet
> *pkt,
> > if (nat_action_info && !create_new_conn) {
> > handle_nat(pkt, conn, zone, ctx->reply, ctx->related);
> > }
> > +
> > + }else if (check_orig_tuple(ct, pkt, ctx, now, &bucket, &conn,
> > + nat_action_info)) {
>
> Sorry, I don't understand the feature very much, so I'm going to ask a
> couple of dumb
> questions :-)
>
> Why is the body of this 'else if' empty? Could you explain a little bit
> more
> in the commit message?
>
The original tuple is another way to an existed conn entry, when NAT is
done on the packet. It's value is mainly for related packets.
I'll add that this to the commit message.
As we discussed offline, you had a concern about conn_update_state()
not being called; this is bug - good catch !
I added the exact same call to the "else if" case as in the "if" case
i.e.
create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now, bucket);
>
> Thanks
>
> > } else {
> > if (ctx->related) {
> > pkt->md.ct_state = CS_INVALID;
> > --
> > 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
