trafgen: proto: Split static protocol definition out of struct proto_hdr

Currently struct proto_hdr is used twofold:

  1) Statically define protocol behavior, i.e. all the *_hdr definitions in
     trafgen_l{2,3,4}.c which map a protocol id/layer to a set of callback
     functions.

  2) For each packet created at parse time the struct is memcpy()'ed
     (including all the static information from 1) and then used to store
     dynamic information at parse/run time.

Thus, struct proto_hdr members such as the proto id, layer and the
pointers callback functions get copied for each created packet (in
addition to the other fields which get changed during parsing). Also,
static/dynamic information get mixed and we e.g. can't make the protocol
definitions const to ensure they'll not get changed by mistake.

Rather than copying the struct proto_hdr for every packet, clearly
separate the two purposes defined above by splitting struct proto_hdr
into two structs:

  1) struct proto_ops for the static (const) protocol behavior definition

  2) struct proto_hdr (reduced) for dynamic information

struct proto_hdr keeps a pointer to the corresponding proto_ops instance
and uses it to execute the corresponding callbacks.

Reference: https://groups.google.com/forum/#!msg/netsniff-ng/20RvwJdh50Y/eMkbmKSaBgAJ
Acked-by: Vadim Kochan <vadim4j@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

6 files changed, 83 insertions, 78 deletions

diff --git a/trafgen_l2.c b/trafgen_l2.c
index 1863332..f09b2a6 100644
--- a/trafgen_l2.c
+++ b/trafgen_l2.c
@@ -47,7 +47,7 @@ static void eth_header_init(struct proto_hdr *hdr)
 	proto_field_set_default_dev_mac(hdr, ETH_SRC_ADDR);
 }
 
-static struct proto_hdr eth_hdr = {
+static const struct proto_ops eth_proto_ops = {
 	.id		= PROTO_ETH,
 	.layer		= PROTO_L2,
 	.header_init	= eth_header_init,
@@ -74,13 +74,13 @@ static void vlan_header_init(struct proto_hdr *hdr)
 
 	proto_header_fields_add(hdr, vlan_fields, array_size(vlan_fields));
 
-	if (lower->id == PROTO_ETH)
+	if (lower->ops->id == PROTO_ETH)
 		lower_etype = proto_field_get_u16(lower, ETH_TYPE);
-	else if (lower->id == PROTO_VLAN)
+	else if (lower->ops->id == PROTO_VLAN)
 		lower_etype = proto_field_get_u16(lower, VLAN_ETYPE);
 
 	proto_field_set_be16(hdr, VLAN_ETYPE, lower_etype);
-	proto_field_set_default_be16(hdr, VLAN_TPID, pid_to_eth(hdr->id));
+	proto_field_set_default_be16(hdr, VLAN_TPID, pid_to_eth(hdr->ops->id));
 }
 
 static void vlan_set_next_proto(struct proto_hdr *hdr, enum proto_id pid)
@@ -89,7 +89,7 @@ static void vlan_set_next_proto(struct proto_hdr *hdr, enum proto_id pid)
 		proto_field_set_be16(hdr, VLAN_ETYPE, pid_to_eth(pid));
 }
 
-static struct proto_hdr vlan_hdr = {
+static const struct proto_ops vlan_proto_ops = {
 	.id		= PROTO_VLAN,
 	.layer		= PROTO_L2,
 	.header_init	= vlan_header_init,
@@ -114,7 +114,7 @@ static void arp_header_init(struct proto_hdr *hdr)
 
 	lower = proto_lower_default_add(hdr, PROTO_ETH);
 
-	if (lower->id == PROTO_ETH) {
+	if (lower->ops->id == PROTO_ETH) {
 		const uint8_t bcast[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
 		proto_field_set_default_bytes(lower, ETH_DST_ADDR, bcast);
@@ -133,7 +133,7 @@ static void arp_header_init(struct proto_hdr *hdr)
 	proto_field_set_default_dev_ipv4(hdr, ARP_TPA);
 }
 
-static struct proto_hdr arp_hdr = {
+static const struct proto_ops arp_proto_ops = {
 	.id		= PROTO_ARP,
 	.layer		= PROTO_L2,
 	.header_init	= arp_header_init,
@@ -161,7 +161,7 @@ static void mpls_set_next_proto(struct proto_hdr *hdr, enum proto_id pid)
 		proto_field_set_default_be32(hdr, MPLS_LAST, 0);
 }
 
-static struct proto_hdr mpls_hdr = {
+static const struct proto_ops mpls_proto_ops = {
 	.id		= PROTO_MPLS,
 	.layer		= PROTO_L2,
 	.header_init	= mpls_header_init,
@@ -170,8 +170,8 @@ static struct proto_hdr mpls_hdr = {
 
 void protos_l2_init(void)
 {
-	proto_header_register(&eth_hdr);
-	proto_header_register(&vlan_hdr);
-	proto_header_register(&arp_hdr);
-	proto_header_register(&mpls_hdr);
+	proto_ops_register(&eth_proto_ops);
+	proto_ops_register(&vlan_proto_ops);
+	proto_ops_register(&arp_proto_ops);
+	proto_ops_register(&mpls_proto_ops);
 }
diff --git a/trafgen_l3.c b/trafgen_l3.c
index 1cdd041..2eabef1 100644
--- a/trafgen_l3.c
+++ b/trafgen_l3.c
@@ -86,7 +86,7 @@ static void ipv4_set_next_proto(struct proto_hdr *hdr, enum proto_id pid)
 	proto_field_set_default_u8(hdr, IP4_PROTO, ip_proto);
 }
 
-static struct proto_hdr ipv4_hdr = {
+static const struct proto_ops ipv4_proto_ops = {
 	.id		= PROTO_IP4,
 	.layer		= PROTO_L3,
 	.header_init	= ipv4_header_init,
@@ -146,7 +146,7 @@ static void ipv6_set_next_proto(struct proto_hdr *hdr, enum proto_id pid)
 	proto_field_set_default_u8(hdr, IP6_NEXT_HDR, ip_proto);
 }
 
-static struct proto_hdr ipv6_hdr = {
+static const struct proto_ops ipv6_proto_ops = {
 	.id             = PROTO_IP6,
 	.layer          = PROTO_L3,
 	.header_init    = ipv6_header_init,
@@ -156,6 +156,6 @@ static struct proto_hdr ipv6_hdr = {
 
 void protos_l3_init(void)
 {
-	proto_header_register(&ipv4_hdr);
-	proto_header_register(&ipv6_hdr);
+	proto_ops_register(&ipv4_proto_ops);
+	proto_ops_register(&ipv6_proto_ops);
 }
diff --git a/trafgen_l4.c b/trafgen_l4.c
index 79c5914..16346db 100644
--- a/trafgen_l4.c
+++ b/trafgen_l4.c
@@ -44,7 +44,7 @@ static void udp_packet_finish(struct proto_hdr *hdr)
 	if (!lower)
 		return;
 
-	switch (lower->id) {
+	switch (lower->ops->id) {
 	case PROTO_IP4:
 		csum = p4_csum((void *) proto_header_ptr(lower), proto_header_ptr(hdr),
 				total_len, IPPROTO_UDP);
@@ -61,7 +61,7 @@ static void udp_packet_finish(struct proto_hdr *hdr)
 	proto_field_set_be16(hdr, UDP_CSUM, bswap_16(csum));
 }
 
-static struct proto_hdr udp_hdr = {
+static const struct proto_ops udp_proto_ops = {
 	.id		= PROTO_UDP,
 	.layer		= PROTO_L4,
 	.header_init	= udp_header_init,
@@ -112,7 +112,7 @@ static void tcp_packet_finish(struct proto_hdr *hdr)
 
 	total_len = pkt->len - hdr->pkt_offset;
 
-	switch (lower->id) {
+	switch (lower->ops->id) {
 	case PROTO_IP4:
 		csum = p4_csum((void *) proto_header_ptr(lower), proto_header_ptr(hdr),
 				total_len, IPPROTO_TCP);
@@ -129,7 +129,7 @@ static void tcp_packet_finish(struct proto_hdr *hdr)
 	proto_field_set_be16(hdr, TCP_CSUM, bswap_16(csum));
 }
 
-static struct proto_hdr tcp_hdr = {
+static const struct proto_ops tcp_proto_ops = {
 	.id		= PROTO_TCP,
 	.layer		= PROTO_L4,
 	.header_init	= tcp_header_init,
@@ -170,7 +170,7 @@ static void icmpv4_packet_finish(struct proto_hdr *hdr)
 	proto_field_set_u16(hdr, ICMPV4_CSUM, bswap_16(csum));
 }
 
-static struct proto_hdr icmpv4_hdr = {
+static const struct proto_ops icmpv4_proto_ops = {
 	.id		= PROTO_ICMP4,
 	.layer		= PROTO_L4,
 	.header_init	= icmpv4_header_init,
@@ -205,7 +205,7 @@ static void icmpv6_packet_finish(struct proto_hdr *hdr)
 
 	total_len = pkt->len - hdr->pkt_offset;
 
-	switch (lower->id) {
+	switch (lower->ops->id) {
 	case PROTO_IP6:
 		csum = p6_csum((void *) proto_header_ptr(lower), proto_header_ptr(hdr),
 				total_len, IPPROTO_ICMPV6);
@@ -218,7 +218,7 @@ static void icmpv6_packet_finish(struct proto_hdr *hdr)
 	proto_field_set_be16(hdr, ICMPV6_CSUM, bswap_16(csum));
 }
 
-static struct proto_hdr icmpv6_hdr = {
+static struct proto_ops icmpv6_proto_ops = {
 	.id		= PROTO_ICMP6,
 	.layer		= PROTO_L4,
 	.header_init	= icmpv6_header_init,
@@ -227,8 +227,8 @@ static struct proto_hdr icmpv6_hdr = {
 
 void protos_l4_init(void)
 {
-	proto_header_register(&udp_hdr);
-	proto_header_register(&tcp_hdr);
-	proto_header_register(&icmpv4_hdr);
-	proto_header_register(&icmpv6_hdr);
+	proto_ops_register(&udp_proto_ops);
+	proto_ops_register(&tcp_proto_ops);
+	proto_ops_register(&icmpv4_proto_ops);
+	proto_ops_register(&icmpv6_proto_ops);
 }
diff --git a/trafgen_parser.y b/trafgen_parser.y
index cd87c12..d643ad2 100644
--- a/trafgen_parser.y
+++ b/trafgen_parser.y
@@ -348,7 +348,7 @@ static void set_dynamic_incdec(uint8_t start, uint8_t stop, uint8_t stepping,
 
 static void proto_add(enum proto_id pid)
 {
-	hdr = proto_header_init(pid);
+	hdr = proto_header_push(pid);
 }
 
 %}
diff --git a/trafgen_proto.c b/trafgen_proto.c
index 4219794..d976c14 100644
--- a/trafgen_proto.c
+++ b/trafgen_proto.c
@@ -27,7 +27,7 @@ struct ctx {
 };
 static struct ctx ctx;
 
-static const struct proto_hdr *registered[__PROTO_MAX];
+static const struct proto_ops *registered_ops[__PROTO_MAX];
 
 struct proto_hdr *proto_lower_header(struct proto_hdr *hdr)
 {
@@ -52,19 +52,18 @@ uint8_t *proto_header_ptr(struct proto_hdr *hdr)
 	return &packet_get(hdr->pkt_id)->payload[hdr->pkt_offset];
 }
 
-static const struct proto_hdr *proto_header_by_id(enum proto_id id)
+static const struct proto_ops *proto_ops_by_id(enum proto_id id)
 {
-	bug_on(id >= __PROTO_MAX);
-	return registered[id];
+	const struct proto_ops *ops = registered_ops[id];
+
+	bug_on(ops->id != id);
+	return ops;
 }
 
-void proto_header_register(struct proto_hdr *hdr)
+void proto_ops_register(const struct proto_ops *ops)
 {
-	bug_on(hdr->id >= __PROTO_MAX);
-	registered[hdr->id] = hdr;
-
-	hdr->fields = NULL;
-	hdr->fields_count = 0;
+	bug_on(ops->id >= __PROTO_MAX);
+	registered_ops[ops->id] = ops;
 }
 
 static void proto_fields_realloc(struct proto_hdr *hdr, size_t count)
@@ -120,52 +119,55 @@ bool proto_field_is_set(struct proto_hdr *hdr, uint32_t fid)
 	return field ? field->is_set : false;
 }
 
-struct proto_hdr *proto_header_init(enum proto_id pid)
+struct proto_hdr *proto_header_push(enum proto_id pid)
 {
 	struct proto_hdr **headers = current_packet()->headers;
-	const struct proto_hdr *hdr = proto_header_by_id(pid);
-	struct proto_hdr *new_hdr;
+	const struct proto_ops *ops = proto_ops_by_id(pid);
+	struct proto_hdr *hdr;
 
 	bug_on(current_packet()->headers_count >= PROTO_MAX_LAYERS);
 
-	new_hdr = xmalloc(sizeof(*new_hdr));
-	memcpy(new_hdr, hdr, sizeof(*new_hdr));
+	hdr = xzmalloc(sizeof(*hdr));
+	hdr->ops = ops;
+	hdr->pkt_id = current_packet_id();
 
-	new_hdr->pkt_id = current_packet_id();
+	if (ops && ops->header_init)
+		ops->header_init(hdr);
 
-	if (new_hdr->header_init)
-		new_hdr->header_init(new_hdr);
+	headers[current_packet()->headers_count++] = hdr;
 
-	headers[current_packet()->headers_count++] = new_hdr;
-	return new_hdr;
+	return hdr;
 }
 
 void proto_header_finish(struct proto_hdr *hdr)
 {
-	if (hdr && hdr->header_finish)
-		hdr->header_finish(hdr);
+	if (hdr && hdr->ops && hdr->ops->header_finish)
+		hdr->ops->header_finish(hdr);
 }
 
-struct proto_hdr *proto_lower_default_add(struct proto_hdr *hdr,
+struct proto_hdr *proto_lower_default_add(struct proto_hdr *upper,
 					  enum proto_id pid)
 {
 	struct proto_hdr *current;
 	size_t headers_count = current_packet()->headers_count;
+	const struct proto_ops *ops;
 
 	if (headers_count > 0) {
 		current = current_packet()->headers[headers_count - 1];
+		ops = current->ops;
 
-		if (current->layer >= proto_header_by_id(pid)->layer)
+		if (ops->layer >= proto_ops_by_id(pid)->layer)
 			goto set_proto;
-		if (current->id == pid)
+		if (ops->id == pid)
 			goto set_proto;
 	}
 
-	current = proto_header_init(pid);
+	current = proto_header_push(pid);
+	ops = current->ops;
 
 set_proto:
-	if (current->set_next_proto)
-		current->set_next_proto(current, hdr->id);
+	if (ops && ops->set_next_proto)
+		ops->set_next_proto(current, upper->ops->id);
 
 	return current;
 }
@@ -415,9 +417,10 @@ void proto_packet_finish(void)
 
 	/* Go down from upper layers to do last calculations (checksum) */
 	for (i = headers_count - 1; i >= 0; i--) {
-		struct proto_hdr *p = headers[i];
+		struct proto_hdr *hdr = headers[i];
+		const struct proto_ops *ops = hdr->ops;
 
-		if (p->packet_finish)
-			p->packet_finish(p);
+		if (ops && ops->packet_finish)
+			ops->packet_finish(hdr);
 	}
 }
diff --git a/trafgen_proto.h b/trafgen_proto.h
index dbba700..9716aa2 100644
--- a/trafgen_proto.h
+++ b/trafgen_proto.h
@@ -29,6 +29,25 @@ enum proto_layer {
 
 struct proto_hdr;
 
+struct proto_ops {
+	enum proto_id id;
+	enum proto_layer layer;
+
+	void (*header_init)(struct proto_hdr *hdr);
+	void (*header_finish)(struct proto_hdr *hdr);
+	void (*packet_finish)(struct proto_hdr *hdr);
+	void (*set_next_proto)(struct proto_hdr *hdr, enum proto_id pid);
+};
+
+struct proto_hdr {
+	const struct proto_ops *ops;
+	uint16_t pkt_offset;
+	uint32_t pkt_id;
+	struct proto_field *fields;
+	size_t fields_count;
+	size_t len;
+};
+
 struct proto_field {
 	uint32_t id;
 	size_t len;
@@ -42,27 +61,10 @@ struct proto_field {
 	struct proto_hdr *hdr;
 };
 
-struct proto_hdr {
-	enum proto_id id;
-	enum proto_layer layer;
-
-	struct proto_hdr *next;
-	uint16_t pkt_offset;
-	uint32_t pkt_id;
-	struct proto_field *fields;
-	size_t fields_count;
-	size_t len;
-
-	void (*header_init)(struct proto_hdr *hdr);
-	void (*header_finish)(struct proto_hdr *hdr);
-	void (*packet_finish)(struct proto_hdr *hdr);
-	void (*set_next_proto)(struct proto_hdr *hdr, enum proto_id pid);
-};
-
 extern void protos_init(const char *dev);
-extern void proto_header_register(struct proto_hdr *hdr);
+extern void proto_ops_register(const struct proto_ops *ops);
 
-extern struct proto_hdr *proto_header_init(enum proto_id pid);
+extern struct proto_hdr *proto_header_push(enum proto_id pid);
 extern void proto_header_finish(struct proto_hdr *hdr);
 extern void proto_packet_finish(void);
 extern struct proto_hdr *proto_lower_default_add(struct proto_hdr *hdr,