l4 port redirect & fix loopback for ip6 packets #112
Conversation
christophefontaine
commented
Dec 19, 2024
christophefontaine
commented
- Fixes the redirection of IPv6 packets
- Prepare the tunnel over UDP, so we can register for a specific UDP port
- simplifies the number of nodes by removing TCP / SCTP dedicated nodes as we don't use them.
christophefontaine
changed the title
christophefontaine
force-pushed
the
ip6_loopback
branch
from
bf4715b to
2dcd062
Compare
We wrongly assumed that the data was still present when prepending an ip header. Rewrite the ip/ip6 header instead. Fixes: e27fab5 ("ip/ip6: post tcp/udp packets to loopback interface") Signed-off-by: Christophe Fontaine <[email protected]>
For VxLan tunnels, we need to register a specific destination udp port. Add a new node and api, and push all non-handled packets back to the management (loopback) interface, as done today. Signed-off-by: Christophe Fontaine <[email protected]>
christophefontaine
force-pushed
the
ip6_loopback
branch
from
2dcd062 to
2b780c9
Compare
| goto next; | ||
| } | ||
|
|
||
| if (mbuf->packet_type & RTE_PTYPE_L3_IPV4) { |
There was a problem hiding this comment.
Relying on RTE_PTYPE_L3_IPV4 is risky (is the tun driver really setting this correctly?).
You'll need something else.
| if (mbuf->packet_type & RTE_PTYPE_L3_IPV4) { | ||
| struct ip_local_mbuf_data *d = ip_local_mbuf_data(mbuf); | ||
| struct rte_ipv4_hdr *ip; | ||
| ip = (struct rte_ipv4_hdr *)rte_pktmbuf_prepend(mbuf, sizeof(*ip)); |
There was a problem hiding this comment.
Where do you make sure there is enough headroom?
There was a problem hiding this comment.
indeed, will fix it
| ip->src_addr = d->src; | ||
| ip->dst_addr = d->dst; | ||
| ip->total_length = rte_cpu_to_be_16(d->len) + sizeof(*ip); | ||
| ip->next_proto_id = d->proto; |
There was a problem hiding this comment.
Some fields don't seem initialized, so you are keeping some stale values from memory.
struct rte_ipv4_hdr {
union {
uint8_t version_ihl; /* 0 1 */
struct {
uint8_t ihl:4; /* 0: 0 1 */
uint8_t version:4; /* 0: 4 1 */
}; /* 0 1 */
}; /* 0 1 */
uint8_t type_of_service; /* 1 1 */
rte_be16_t total_length; /* 2 2 */
rte_be16_t packet_id; /* 4 2 */
rte_be16_t fragment_offset; /* 6 2 */
uint8_t time_to_live; /* 8 1 */
uint8_t next_proto_id; /* 9 1 */
rte_be16_t hdr_checksum; /* 10 2 */
rte_be32_t src_addr; /* 12 4 */
rte_be32_t dst_addr; /* 16 4 */
/* size: 20, cachelines: 1, members: 10 */
/* last cacheline: 20 bytes */
} __attribute__((__aligned__(2)));
There was a problem hiding this comment.
Will also fix for ipv6 hdr.
| ip->dst_addr = d->dst; | ||
| ip->payload_len = rte_cpu_to_be_16(d->len); | ||
| ip->hop_limits = d->hop_limit; | ||
| ip->proto = d->proto; |
There was a problem hiding this comment.
Idem, not initialized fields.
| @@ -134,6 +134,7 @@ static void iface_loopback_poll(evutil_socket_t, short reason, void *ev_iface) { | |||
|
|
|||
| // packet sent from linux tun iface, no need to compute checksum; | |||
| mbuf->ol_flags = RTE_MBUF_F_RX_IP_CKSUM_GOOD; | |||
| mbuf->packet_type = data[0] == 6 ? RTE_PTYPE_L3_IPV6 : RTE_PTYPE_L3_IPV4; | |||
There was a problem hiding this comment.
Ugh, ok.
I guess the previous patch relies on this.
This must be documented, or a different mechanism is needed.
There was a problem hiding this comment.
I wished to avoid setting unnecessary data if the packet type was already defined by the drivers.
The other option is to add a family field in the buffer metadata for both ip_local_mbuf_data and ip6_local_mbuf_data to define which kind of header we should fill in.
I found it uglier as the 2 priv data types are defined in different modules.
GR_MBUF_PRIV_DATA_TYPE(ip_local_mbuf_data, {
+ uint8_t family;
ip4_addr_t src;
ip4_addr_t dst;
uint16_t len;
uint16_t vrf_id;
uint8_t proto;
uint8_t ttl;
});
GR_MBUF_PRIV_DATA_TYPE(ip6_local_mbuf_data, {
+ uint8_t family;
struct rte_ipv6_addr src;
struct rte_ipv6_addr dst;
uint16_t len;
uint8_t hop_limit;
uint8_t proto;
});
@rjarry Any opinion about that ?
Should we define instead a common header for both local_mbuf_data instead ?
GR_MBUF_PRIV_DATA_TYPE(ip46_local_mbuf_data, {
uint8_t family;
union {
struct rte_ipv6_addr v6;
ip4_addr_t v4;
} src;
union {
struct rte_ipv6_addr v6;
ip4_addr_t v4;
} dst;
uint16_t len;
union {
uint8_t hop_limit;
uint8_t ttl;
};
uint8_t proto;
});
|
|
||
| static void l4_input_local_register(void) { | ||
| ip_input_local_add_proto(IPPROTO_UDP, "l4_input_local"); | ||
| ip6_input_local_add_proto(IPPROTO_TCP, "l4_input_local"); |
There was a problem hiding this comment.
ouch .... should register for both ip4/udp ip6/udp and ip4/tcp ip4/tcp
