Skip to content

Commit

Permalink
bpf: nat: consistently use has_l4_header in IPv4 SNAT path
Browse files Browse the repository at this point in the history
We currently don't support IPv4 fragmentation in the SNAT path (or more
precisely, we don't have support for loading the L4 ports of a fragment).

But we already have two spots that correctly check whether the packet has
a L4 header (or only requires L3 SNAT). Make this consistent across the
whole flow, so that it also gets applied in the CT lookup.

Passing this through to snat_v4_nat_handle_icmp_frag_needed() also avoids a
revalidation of the outer header.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
  • Loading branch information
julianwiedmann committed May 29, 2023
1 parent 1b19e5f commit d583be6
Showing 1 changed file with 7 additions and 10 deletions.
17 changes: 7 additions & 10 deletions bpf/lib/nat.h
Original file line number Diff line number Diff line change
Expand Up @@ -938,16 +938,15 @@ static __always_inline bool snat_v4_prepare_state(struct __ctx_buff *ctx,
}

static __always_inline __maybe_unused int
snat_v4_nat_handle_icmp_frag_needed(struct __ctx_buff *ctx, __u64 off)
snat_v4_nat_handle_icmp_frag_needed(struct __ctx_buff *ctx, __u64 off,
bool has_l4_header)
{
struct ipv4_ct_tuple tuple = {};
struct ipv4_nat_entry *state;
struct iphdr iphdr;
__u32 icmpoff = off + sizeof(struct icmphdr);
__be16 identifier;
__u8 type;
void *data, *data_end;
struct iphdr *ip4;
int ret;

/* According to the RFC 5508, any networking equipment that is
Expand Down Expand Up @@ -1007,17 +1006,14 @@ snat_v4_nat_handle_icmp_frag_needed(struct __ctx_buff *ctx, __u64 off)
if (IS_ERR(ret))
return ret;

if (!revalidate_data(ctx, &data, &data_end,
&ip4))
return DROP_INVALID;
/* Switch back to the outer header. */
tuple.nexthdr = IPPROTO_ICMP;
/* Reset so no l4 NAT is done in snat_v4_rewrite_egress. We don't need
* it because we are handling ICMP_DEST_UNREACH which doesn't have id.
*/
tuple.sport = state->to_sport;

return snat_v4_rewrite_egress(ctx, &tuple, state, off, ipv4_has_l4_header(ip4));
return snat_v4_rewrite_egress(ctx, &tuple, state, off, has_l4_header);
}

static __always_inline __maybe_unused int
Expand All @@ -1033,7 +1029,7 @@ snat_v4_nat(struct __ctx_buff *ctx, const struct ipv4_nat_target *target, __s8 *
__be16 dport;
} l4hdr;
bool icmp_echoreply = false;
bool has_l4_header = true;
bool has_l4_header;
int ct_action = ACTION_UNSPEC;
__u64 off;
int ret;
Expand All @@ -1044,6 +1040,7 @@ snat_v4_nat(struct __ctx_buff *ctx, const struct ipv4_nat_target *target, __s8 *
return DROP_INVALID;

snat_v4_init_tuple(ip4, NAT_DIR_EGRESS, &tuple);
has_l4_header = ipv4_has_l4_header(ip4);

off = ((void *)ip4 - data) + ipv4_hdrlen(ip4);
switch (tuple.nexthdr) {
Expand Down Expand Up @@ -1076,7 +1073,7 @@ snat_v4_nat(struct __ctx_buff *ctx, const struct ipv4_nat_target *target, __s8 *
case ICMP_DEST_UNREACH:
if (icmphdr.code != ICMP_FRAG_NEEDED)
return DROP_UNKNOWN_ICMP_CODE;
return snat_v4_nat_handle_icmp_frag_needed(ctx, off);
return snat_v4_nat_handle_icmp_frag_needed(ctx, off, has_l4_header);
default:
return DROP_NAT_UNSUPP_PROTO;
}
Expand All @@ -1094,7 +1091,7 @@ snat_v4_nat(struct __ctx_buff *ctx, const struct ipv4_nat_target *target, __s8 *
if (ret < 0)
return ret;

return snat_v4_rewrite_egress(ctx, &tuple, state, off, ipv4_has_l4_header(ip4));
return snat_v4_rewrite_egress(ctx, &tuple, state, off, has_l4_header);
}

static __always_inline __maybe_unused int
Expand Down

0 comments on commit d583be6

Please sign in to comment.