Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First working prototype - DHCPv4 #11

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

First working prototype - DHCPv4 #11

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
83b9795
Adding custom memcpy and memset functions to pass verifier checks
Oct 24, 2021
8d91b14
Upstream direction works with QinQ packets only
Oct 25, 2021
17f1da5
First working prototype (IPv4 DHCP relay works in both directions)
Nov 5, 2021
813cfa2
Working prototype (IPv4) with interface name in Option 82 Circuit ID
Nov 9, 2021
f266202
Improve readability of write_dhcp_option_82()
tohojo Nov 12, 2021
b970472
dhcp_user_xdp: Fix indentation of switch cases
tohojo Nov 12, 2021
fd2b750
dhcp_user_xdp: Add -v option to enable verbose logging
tohojo Nov 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dhcp-relay/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ USER_TARGETS := dhcp_user_xdp
BPF_TARGETS :=dhcp_kern_xdp
EXTRA_DEPS := dhcp-relay.h
#EXTRA_CFLAGS := $(if $(IPV6),-DIPV6)
EXTRA_CFLAGS := -fno-builtin

LIB_DIR = ../lib

Expand Down
6 changes: 4 additions & 2 deletions dhcp-relay/dhcp-relay.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,21 @@
#define DHO_DHCP_AGENT_OPTIONS 82
#define RAI_CIRCUIT_ID 1
#define RAI_REMOTE_ID 2
#define RAI_OPTION_LEN 40
#define RAI_OPTION_LEN IF_NAMESIZE
#define VLAN_ASCII_MAX 4 /* Max bytes needed to store VLAN in ASCII format */

#define DHCP_SERVER_PORT 67
#define DHCP_CLIENT_PORT 68
#define DHCP_REQUEST 1
#define DHCP_REPLY 2

#define MAX_LOOPS 40

/* Structure for sub-options in option 82 */
struct sub_option {
__u8 option_id;
__u8 len;
char val[IF_NAMESIZE];
char val[RAI_OPTION_LEN];
};

/*structure for dhcp option 82 */
Expand Down
138 changes: 88 additions & 50 deletions dhcp-relay/dhcp_kern_xdp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
#include <xdp/context_helpers.h>
#include "dhcp-relay.h"

#define bpf_printk(fmt, ...) \
({ \
char ____fmt[] = fmt; \
bpf_trace_printk(____fmt, sizeof(____fmt), \
##__VA_ARGS__); \
})

/*
* This map is for storing the DHCP relay configuration, including:
*
Expand Down Expand Up @@ -49,76 +56,104 @@ struct {
__uint(max_entries, 16384);
} client_vlans SEC(".maps");

void memcpy_var(void *to, void *from, __u64 len) {
__u8 *t8 = to, *f8 = from;
int i;

for (i = 0; i < len && i < MAX_LOOPS; i++) {
*t8++ = *f8++;
}

}

void memset_var(void *d, __u8 c, __u64 len) {
__u8 *d8 = d;
int i;

for (i = 0; i < len && i < MAX_LOOPS; i++) {
*d8++ = c;
}

}

/* Inserts DHCP option 82 into the received DHCP packet
* at the specified offset.
*/
static __always_inline int write_dhcp_option_82(void *ctx, int offset,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, better to make this one the global function; that requires turning 'dev' into a pointer argument (pass-by-value is not supported). I'd also suggest renaming offset to pkt_offset, as we already have way too many offsets in this code...

struct collect_vlans *vlans, char *dev) {

struct dhcp_option_82 option;


static __u8 buf[RAI_OPTION_LEN];

option.t = DHO_DHCP_AGENT_OPTIONS;
option.len = sizeof (struct sub_option) + sizeof (struct sub_option);
option.circuit_id.option_id = RAI_CIRCUIT_ID;
option.circuit_id.len = sizeof(option.circuit_id.val);
option.circuit_id.len = sizeof (option.circuit_id.val);
option.remote_id.option_id = RAI_REMOTE_ID;
option.remote_id.len = sizeof (option.remote_id.val);

/* Initialize val arrays */
memset_var(option.circuit_id.val, 0, sizeof (option.circuit_id.val));
memset_var(option.remote_id.val, '*', sizeof (option.remote_id.val));
//memset(option.circuit_id.val, 0, sizeof (option.circuit_id.val));
//memset(option.remote_id.val, '*', sizeof (option.remote_id.val));

/* Reconstruct VLAN device name
* Convert VLAN tags to ASCII from right to left, starting with
* inner VLAN tag.
* Device name is 16 characters long and prepended with dash, e.g.:
* ----ens6f0.83.20
* We avoid null bytes to ensure compatibility with DHCP servers that
* interpret null as a string terminator.
* Device name is up to 16 characters long - remaining buffer space
* contains null bytes.
*/

char buf[IF_NAMESIZE];
memset(buf, '-', sizeof (buf));

memset(buf, 0, sizeof (buf));

int c = VLAN_ASCII_MAX; /* We will need 4 bytes at most */
int i = RAI_OPTION_LEN - 1;

int c = VLAN_ASCII_MAX; /* We will need 4 bytes at most */
int i = IF_NAMESIZE - 1;
__u16 inner_vlan = vlans->id[1];
__u16 outer_vlan = vlans->id[0];


/* Convert inner VLAN to ASCII */
#pragma unroll VLAN_ASCII_MAX
for (c = VLAN_ASCII_MAX; c > 0; c--) {
buf[i--] = (inner_vlan % 10) + '0';
inner_vlan /= 10;
if (inner_vlan == 0) {
break;
}
}

buf[i--] = '.';


/* Convert outer VLAN to ASCII */
#pragma unroll VLAN_ASCII_MAX
for (c = VLAN_ASCII_MAX; c > 0; c--) {
buf[i--] = (outer_vlan % 10) + '0';
outer_vlan /= 10;
if (outer_vlan == 0) {
break;
}
}


buf[i--] = '.';

for (c = IF_NAMESIZE - 1; c >= 0; c--) {

if (dev[c] != 0) {
/* Append interface name */
#pragma unroll RAI_OPTION_LEN
for (c = RAI_OPTION_LEN - 1; c >= 0; c--) {
if (dev[c] != 0)
buf[i--] = dev[c];
}

if (i < 0) {
break;
}

}

if(sizeof(option.circuit_id.val) == sizeof(buf)) {
memcpy(option.circuit_id.val, buf, sizeof(buf));
if (i < 0)
break;
}

/* Initialize remote ID */
memset(option.remote_id.val, 0, sizeof(option.remote_id.val));
option.remote_id.option_id = RAI_REMOTE_ID;
option.remote_id.len = sizeof(option.remote_id.val);
i++;

/* Copy resulting interface name to circuit_id */
if (sizeof (option.circuit_id.val) == sizeof (buf)) {
memcpy_var(option.circuit_id.val, buf + i, sizeof (buf) - i);
//memcpy_var(option.circuit_id.val, buf, sizeof (buf));
}

return xdp_store_bytes(ctx, offset, &option, sizeof (option), 0);
}
Expand Down Expand Up @@ -166,13 +201,6 @@ static __always_inline int calc_ip_csum(struct iphdr *oldip, struct iphdr *ip,
*/
//static __u8 buf[static_offset + VLAN_MAX_DEPTH * sizeof (struct vlan_hdr)];

#define bpf_printk(fmt, ...) \
({ \
char ____fmt[] = fmt; \
bpf_trace_printk(____fmt, sizeof(____fmt), \
##__VA_ARGS__); \
})

/* XDP program for parsing the DHCP packet and inserting the option 82*/
SEC(XDP_PROG_SEC)
int xdp_dhcp_relay(struct xdp_md *ctx) {
Expand All @@ -183,10 +211,10 @@ int xdp_dhcp_relay(struct xdp_md *ctx) {
int res = bpf_xdp_adjust_tail(ctx, delta);
if (res != 0) {
bpf_printk("Cannot tail extend packet, delta %i - error code %i", delta, res);
return XDP_ABORTED;
return XDP_PASS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a packet we figured we should handle, but that failed; so what is XDP_PASS supposed to do with it? Better to drop...

}

bpf_printk("Tail extended packet by %i bytes", delta);
//bpf_printk("Tail extended packet by %i bytes", delta);

void *data_end = (void *) (long) ctx->data_end;
void *data = (void *) (long) ctx->data;
Expand Down Expand Up @@ -217,27 +245,34 @@ int xdp_dhcp_relay(struct xdp_md *ctx) {
int key = 0;
int len = 0;

if (data + 1 > data_end)
return XDP_ABORTED;
if (data + 1 > data_end) {
bpf_printk("Empty packet\n");
goto out;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unneded braces


nh.pos = data;
ether_type = parse_ethhdr_vlan(&nh, data_end, &eth, &vlans);
/* check for valid ether type */
if (ether_type < 0) {
bpf_printk("Cannot determine ethertype");
rc = XDP_ABORTED;
bpf_printk("Cannot determine ethertype\n");
goto out;
}

if (ether_type != bpf_htons(ETH_P_IP)) {
bpf_printk("Ethertype %#x is not ETH_P_IP", bpf_ntohs(ether_type));
//bpf_printk("Ethertype %x is not ETH_P_IP\n", bpf_ntohs(ether_type));
goto out;
}

bpf_printk("Ethertype %x", bpf_ntohs(ether_type));

bpf_printk("Ethertype %x\n", bpf_ntohs(ether_type));
/* Check at least two vlan tags are present */
if (vlans.id[0] == 0) {
bpf_printk("No outer VLAN tag set\n");
goto out;
}

if (vlans.id[1] == 0) {
bpf_printk("No VLAN tags set");
bpf_printk("No inner VLAN tag set\n");
goto out;
}

Expand All @@ -264,6 +299,8 @@ int xdp_dhcp_relay(struct xdp_md *ctx) {

/* Increase UDP length header */
udp->len += bpf_htons(delta);

udp->check = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you're modifying the packet, but any goto out statements below will end up returning XDP_PASS, passing up a potentially partially-modified packet. rc should be changed to XDP_DROP at the point of the first packet modification.


/* Read DHCP server IP from config map */
key = 0;
Expand Down Expand Up @@ -294,6 +331,7 @@ int xdp_dhcp_relay(struct xdp_md *ctx) {
// goto out;

/* Increment offset by 4 bytes for each VLAN (to accomodate VLAN headers */
#pragma unroll VLAN_MAX_DEPTH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed

for (i = 0; i < VLAN_MAX_DEPTH; i++) {
if (vlans.id[i]) {

Expand Down Expand Up @@ -426,7 +464,7 @@ int xdp_dhcp_relay(struct xdp_md *ctx) {
bpf_printk("Could not write DHCP option 82 at offset %i", option_offset);
return XDP_ABORTED;
}

/* Set END option */

/* Verifier check */
Expand Down