From 17c13c975369f49cb8f6a5bbd868be816077d2e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarom=C3=ADr=20Smr=C4=8Dek?= <4plague@gmail.com> Date: Tue, 21 Nov 2023 17:09:51 +0100 Subject: [PATCH] Added type conversion compiler checks --- Dockerfile | 1 + docs/development/building.md | 1 + hack/dpdk_22_11_ethdev_conversion.patch | 26 +++++++++++ include/dp_conf.h | 2 +- include/dp_flow.h | 2 +- include/dp_mbuf_dyn.h | 4 +- include/dp_vnf.h | 8 ++-- include/grpc/dp_grpc_api.h | 20 ++++---- include/grpc/dp_grpc_responder.h | 7 ++- include/monitoring/dp_monitoring.h | 6 +-- include/rte_flow/dp_rte_flow_helpers.h | 4 +- include/rte_flow/dp_rte_flow_init.h | 4 +- meson.build | 3 +- src/dp_argparse.c | 2 +- src/dp_conf.c | 4 +- src/dp_flow.c | 7 +-- src/dp_hairpin.c | 4 +- src/dp_lb.c | 2 +- src/dp_log.c | 4 +- src/dp_lpm.c | 6 +-- src/dp_netlink.c | 10 ++-- src/dp_port.c | 10 ++-- src/dp_service.c | 3 +- src/dp_telemetry.c | 2 +- src/dp_util.c | 4 +- src/dp_virtsvc.c | 9 ++-- src/dp_vnf.c | 8 ++-- src/dpdk_layer.c | 5 +- src/grpc/dp_async_grpc.cpp | 54 ++++++++++++++-------- src/grpc/dp_grpc_impl.c | 12 +++-- src/grpc/dp_grpc_responder.c | 9 +++- src/monitoring/dp_monitoring.c | 2 +- src/monitoring/dp_pcap.c | 4 +- src/nodes/cls_node.c | 4 +- src/nodes/dhcp_node.c | 30 +++++++----- src/nodes/dhcpv6_node.c | 21 +++++---- src/nodes/packet_relay_node.c | 2 +- src/nodes/rx_periodic_node.c | 12 ++--- src/nodes/virtsvc_node.c | 10 ++-- src/rte_flow/dp_rte_flow_init.c | 4 +- src/rte_flow/dp_rte_flow_traffic_forward.c | 3 +- tools/dp_grpc_client.cpp | 2 +- 42 files changed, 203 insertions(+), 134 deletions(-) create mode 100644 hack/dpdk_22_11_ethdev_conversion.patch diff --git a/Dockerfile b/Dockerfile index cceae3f16..33e54ef1c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -54,6 +54,7 @@ COPY hack/*.patch hack/ RUN cd $DPDK_DIR && patch -p1 < ../hack/dpdk_22_11_gcc12.patch RUN cd $DPDK_DIR && patch -p1 < ../hack/dpdk_22_11_log.patch RUN cd $DPDK_DIR && patch -p1 < ../hack/dpdk_22_11_telemetry_key.patch +RUN cd $DPDK_DIR && patch -p1 < ../hack/dpdk_22_11_ethdev_conversion.patch # Compile DPDK RUN cd $DPDK_DIR && meson setup -Dmax_ethports=132 -Dplatform=generic -Ddisable_drivers=common/dpaax,\ diff --git a/docs/development/building.md b/docs/development/building.md index 6085545f6..336090f1f 100644 --- a/docs/development/building.md +++ b/docs/development/building.md @@ -35,6 +35,7 @@ cd dpdk-stable-22.11.3 patch -p1 < ../net-dpservice/hack/dpdk_22_11_gcc12.patch patch -p1 < ../net-dpservice/hack/dpdk_22_11_log.patch patch -p1 < ../net-dpservice/hack/dpdk_22_11_telemetry_key.patch +patch -p1 < ../net-dpservice/hack/dpdk_22_11_ethdev_conversion.patch meson setup build ninja -C build sudo ninja -C build install diff --git a/hack/dpdk_22_11_ethdev_conversion.patch b/hack/dpdk_22_11_ethdev_conversion.patch new file mode 100644 index 000000000..023ec21b3 --- /dev/null +++ b/hack/dpdk_22_11_ethdev_conversion.patch @@ -0,0 +1,26 @@ +diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c +index 5d5e18db1e..d8908b722a 100644 +--- a/lib/ethdev/rte_ethdev.c ++++ b/lib/ethdev/rte_ethdev.c +@@ -387,7 +387,7 @@ eth_is_valid_owner_id(uint64_t owner_id) + return 1; + } + +-uint64_t ++uint16_t + rte_eth_find_next_owned_by(uint16_t port_id, const uint64_t owner_id) + { + port_id = rte_eth_find_next(port_id); +diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h +index c129ca1eaf..a92fe1259e 100644 +--- a/lib/ethdev/rte_ethdev.h ++++ b/lib/ethdev/rte_ethdev.h +@@ -2059,7 +2059,7 @@ struct rte_eth_dev_owner { + * @return + * Next valid port ID owned by owner_id, RTE_MAX_ETHPORTS if there is none. + */ +-uint64_t rte_eth_find_next_owned_by(uint16_t port_id, ++uint16_t rte_eth_find_next_owned_by(uint16_t port_id, + const uint64_t owner_id); + + /** diff --git a/include/dp_conf.h b/include/dp_conf.h index f2a3f1303..ac9326ef0 100644 --- a/include/dp_conf.h +++ b/include/dp_conf.h @@ -25,7 +25,7 @@ struct dp_conf_virtual_services { #endif struct dp_conf_dhcp_dns { - int len; + uint8_t len; uint8_t *array; }; diff --git a/include/dp_flow.h b/include/dp_flow.h index cf9135d1b..90750d0e9 100644 --- a/include/dp_flow.h +++ b/include/dp_flow.h @@ -127,7 +127,7 @@ int dp_build_flow_key(struct flow_key *key /* out */, struct rte_mbuf *m /* in * void dp_invert_flow_key(const struct flow_key *key /* in */, struct flow_key *inv_key /* out */); int dp_flow_init(int socket_id); void dp_flow_free(void); -void dp_process_aged_flows(int port_id); +void dp_process_aged_flows(uint16_t port_id); void dp_process_aged_flows_non_offload(void); void dp_free_flow(struct dp_ref *ref); void dp_free_network_nat_port(const struct flow_value *cntrack); diff --git a/include/dp_mbuf_dyn.h b/include/dp_mbuf_dyn.h index 6f8283d81..89c582431 100644 --- a/include/dp_mbuf_dyn.h +++ b/include/dp_mbuf_dyn.h @@ -63,8 +63,6 @@ struct dp_flow { rte_be32_t nat_addr; uint16_t nat_port; - uint8_t nxt_hop; - uint8_t l4_type; union { struct { @@ -88,6 +86,8 @@ struct dp_flow { uint32_t dst_vni; } tun_info; + uint16_t nxt_hop; // this can be brought down to only one byte (low DP_MAX_PORTS) + struct flow_value *conntrack; #ifdef ENABLE_VIRTSVC struct dp_virtsvc *virtsvc; diff --git a/include/dp_vnf.h b/include/dp_vnf.h index e32dd0898..9a2a71a16 100644 --- a/include/dp_vnf.h +++ b/include/dp_vnf.h @@ -28,7 +28,7 @@ enum dp_vnf_type { struct dp_vnf_prefix { uint32_t ip; - uint16_t length; + uint8_t length; }; struct dp_vnf { @@ -42,13 +42,13 @@ int dp_vnf_init(int socket_id); void dp_vnf_free(void); int dp_add_vnf(const uint8_t ul_addr6[DP_VNF_IPV6_ADDR_SIZE], enum dp_vnf_type type, - uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint16_t prefix_len); + uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint8_t prefix_len); const struct dp_vnf *dp_get_vnf(const uint8_t ul_addr6[DP_VNF_IPV6_ADDR_SIZE]); int dp_del_vnf(const uint8_t ul_addr6[DP_VNF_IPV6_ADDR_SIZE]); -bool dp_vnf_lbprefix_exists(uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint16_t prefix_len); +bool dp_vnf_lbprefix_exists(uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint8_t prefix_len); -int dp_del_vnf_by_value(enum dp_vnf_type type, uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint16_t prefix_len); +int dp_del_vnf_by_value(enum dp_vnf_type type, uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint8_t prefix_len); int dp_list_vnf_alias_prefixes(uint16_t port_id, enum dp_vnf_type type, struct dp_grpc_responder *responder); diff --git a/include/grpc/dp_grpc_api.h b/include/grpc/dp_grpc_api.h index 423226145..3e3eb4a81 100644 --- a/include/grpc/dp_grpc_api.h +++ b/include/grpc/dp_grpc_api.h @@ -99,13 +99,13 @@ struct dpgrpc_address { struct dpgrpc_prefix { struct dpgrpc_address addr; - uint32_t length; + uint8_t length; char iface_id[DP_IFACE_ID_MAX_LEN]; }; struct dpgrpc_route { struct dpgrpc_address pfx_addr; - uint32_t pfx_length; + uint8_t pfx_length; uint32_t vni; struct dpgrpc_address trgt_addr; uint32_t trgt_vni; @@ -181,8 +181,8 @@ struct dpgrpc_capture_interface { struct dpgrpc_capture { uint8_t dst_addr6[DP_VNF_IPV6_ADDR_SIZE]; uint8_t interface_count; - uint32_t udp_src_port; - uint32_t udp_dst_port; + uint16_t udp_src_port; + uint16_t udp_dst_port; struct dpgrpc_capture_interface interfaces[DP_CAPTURE_MAX_PORT_NUM]; bool is_active; }; @@ -192,7 +192,7 @@ struct dpgrpc_capture_stop { }; struct dpgrpc_request { - uint16_t type; // enum dpgrpc_request_type + enum dpgrpc_request_type type; union { struct dpgrpc_iface add_iface; struct dpgrpc_iface_id del_iface; @@ -251,12 +251,12 @@ struct dpgrpc_vni_in_use { }; struct dpgrpc_reply { - uint8_t type; // copied enum dpgrpc_request_type - uint8_t is_chained; - uint16_t msg_count; - uint32_t err_code; + enum dpgrpc_request_type type; // copied enum dpgrpc_request_type + bool is_chained; + uint16_t msg_count; + uint32_t err_code; union { - uint8_t messages[0]; // used for multiresponse mode + uint8_t messages[0]; // used for multiresponse mode struct dpgrpc_ul_addr ul_addr; struct dpgrpc_iface iface; struct dpgrpc_vf_pci vf_pci; diff --git a/include/grpc/dp_grpc_responder.h b/include/grpc/dp_grpc_responder.h index 95a0d77c3..922a94cbc 100644 --- a/include/grpc/dp_grpc_responder.h +++ b/include/grpc/dp_grpc_responder.h @@ -20,12 +20,11 @@ struct dp_grpc_responder { struct dpgrpc_reply *rep; size_t rep_max_size; size_t msg_size; - int rep_capacity; - int rep_msgcount; + size_t rep_capacity; + uint16_t rep_msgcount; }; -// returns request's type -uint16_t dp_grpc_init_responder(struct dp_grpc_responder *responder, struct rte_mbuf *req_mbuf); +enum dpgrpc_request_type dp_grpc_init_responder(struct dp_grpc_responder *responder, struct rte_mbuf *req_mbuf); static inline void *dp_grpc_single_reply(struct dp_grpc_responder *responder) { diff --git a/include/monitoring/dp_monitoring.h b/include/monitoring/dp_monitoring.h index ac17831d6..c0de06978 100644 --- a/include/monitoring/dp_monitoring.h +++ b/include/monitoring/dp_monitoring.h @@ -33,14 +33,14 @@ struct dp_event_msg { struct dp_capture_hdr_config { uint8_t capture_node_ipv6_addr[16]; - uint32_t capture_udp_src_port; - uint32_t capture_udp_dst_port; + uint16_t capture_udp_src_port; + uint16_t capture_udp_dst_port; }; void dp_process_event_msg(struct rte_mbuf *m); -void dp_set_capture_hdr_config(uint8_t *addr, uint32_t udp_src_port, uint32_t udp_dst_port); +void dp_set_capture_hdr_config(uint8_t *addr, uint16_t udp_src_port, uint16_t udp_dst_port); const struct dp_capture_hdr_config *dp_get_capture_hdr_config(void); void dp_set_capture_enabled(bool enabled); diff --git a/include/rte_flow/dp_rte_flow_helpers.h b/include/rte_flow/dp_rte_flow_helpers.h index c8ac1b5b5..fbf033f9e 100644 --- a/include/rte_flow/dp_rte_flow_helpers.h +++ b/include/rte_flow/dp_rte_flow_helpers.h @@ -509,7 +509,9 @@ void dp_set_flow_age_action(struct rte_flow_action *action, struct rte_flow_action_age *flow_age_action, uint32_t timeout, void *age_context) { - flow_age_action->timeout = timeout; + // timeout has only 24 bits + // mask should be safe, this is a defined constant unless in testing mode + flow_age_action->timeout = timeout & 0x00FFFFFF; flow_age_action->reserved = 0; flow_age_action->context = age_context; action->type = RTE_FLOW_ACTION_TYPE_AGE; diff --git a/include/rte_flow/dp_rte_flow_init.h b/include/rte_flow/dp_rte_flow_init.h index dad2f7255..b02386726 100644 --- a/include/rte_flow/dp_rte_flow_init.h +++ b/include/rte_flow/dp_rte_flow_init.h @@ -10,10 +10,10 @@ extern "C" { #include #include "dp_port.h" -int dp_install_isolated_mode_ipip(int port_id, uint8_t proto_id); +int dp_install_isolated_mode_ipip(uint16_t port_id, uint8_t proto_id); #ifdef ENABLE_VIRTSVC -int dp_install_isolated_mode_virtsvc(int port_id, uint8_t proto_id, const uint8_t svc_ipv6[16], uint16_t svc_port); +int dp_install_isolated_mode_virtsvc(uint16_t port_id, uint8_t proto_id, const uint8_t svc_ipv6[16], uint16_t svc_port); #endif #ifdef __cplusplus diff --git a/meson.build b/meson.build index ce45073f4..b9ec01c99 100644 --- a/meson.build +++ b/meson.build @@ -12,7 +12,8 @@ if get_option('warning_level').to_int() >= 2 common_flags = [ # -Werror provided by 'werror=true' in default project options # -Wall -Wextra provided by 'warning_level=2' in default project options - '-Wunused', '-Wshadow', '-Wundef', '-Wcast-qual', '-Wwrite-strings', '-Wpointer-arith', + '-Wunused', '-Wshadow', '-Wundef', '-Wcast-qual', '-Wwrite-strings', '-Wpointer-arith', + '-Wconversion', '-Wno-sign-conversion' ] cflags += common_flags + [ '-Wbad-function-cast', '-Wmissing-declarations', '-Wstrict-prototypes', '-Wmissing-prototypes', diff --git a/src/dp_argparse.c b/src/dp_argparse.c index 5349f570e..69f5a6858 100644 --- a/src/dp_argparse.c +++ b/src/dp_argparse.c @@ -42,7 +42,7 @@ int dp_argparse_enum(const char *arg, int *dst, const char *choices[], size_t ch { for (size_t i = 0; i < choice_count; ++i) { if (!strcmp(choices[i], arg)) { - *dst = i; + *dst = (int)i; return DP_OK; } } diff --git a/src/dp_conf.c b/src/dp_conf.c index 1a6f04c67..6838d29a3 100644 --- a/src/dp_conf.c +++ b/src/dp_conf.c @@ -136,7 +136,7 @@ static int add_virtsvc(uint8_t proto, const char *str) DP_EARLY_ERR("Invalid virtual service IPv4 port '%s'", tok); return DP_ERROR; } - from_port = htons(longport); + from_port = htons((uint16_t)longport); tok = strtok(NULL, ","); if (!tok) { @@ -158,7 +158,7 @@ static int add_virtsvc(uint8_t proto, const char *str) DP_EARLY_ERR("Invalid virtual service IPv6 port '%s'", tok); return DP_ERROR; } - to_port = htons(longport); + to_port = htons((uint16_t)longport); // prevent virtual/service address duplicates for (int i = 0; i < virtual_services.nb_entries; ++i) { diff --git a/src/dp_flow.c b/src/dp_flow.c index ff5814963..73ba0bcab 100644 --- a/src/dp_flow.c +++ b/src/dp_flow.c @@ -305,7 +305,7 @@ int dp_destroy_rte_flow_agectx(struct flow_age_ctx *agectx) return DP_OK; } -void dp_process_aged_flows(int port_id) +void dp_process_aged_flows(uint16_t port_id) { int total, fetched; struct flow_age_ctx *agectx; @@ -485,10 +485,11 @@ hash_sig_t dp_get_conntrack_flow_hash_value(const struct flow_key *key) int dp_add_rte_age_ctx(struct flow_value *cntrack, struct flow_age_ctx *ctx) { - for (size_t i = 0; i < sizeof(cntrack->rte_age_ctxs); ++i) { + static_assert(RTE_DIM(cntrack->rte_age_ctxs) <= UINT8_MAX, "Conntrack age context storage is too large"); + for (size_t i = 0; i < RTE_DIM(cntrack->rte_age_ctxs); ++i) { if (!cntrack->rte_age_ctxs[i]) { cntrack->rte_age_ctxs[i] = ctx; - ctx->ref_index_in_cntrack = i; + ctx->ref_index_in_cntrack = (uint8_t)i; return DP_OK; } } diff --git a/src/dp_hairpin.c b/src/dp_hairpin.c index 45d61c00e..8c24bd4e3 100644 --- a/src/dp_hairpin.c +++ b/src/dp_hairpin.c @@ -8,8 +8,8 @@ static int setup_hairpin_rx_tx_queues(uint16_t port_id, uint16_t peer_port_id, - uint8_t hairpin_queue_id, - uint8_t peer_hairpin_queue_id) + uint16_t hairpin_queue_id, + uint16_t peer_hairpin_queue_id) { int ret; diff --git a/src/dp_lb.c b/src/dp_lb.c index a3a8c1561..e37458d40 100644 --- a/src/dp_lb.c +++ b/src/dp_lb.c @@ -246,7 +246,7 @@ uint8_t *dp_lb_get_backend_ip(uint32_t ol_ip, uint32_t vni, rte_be16_t port, uin if (pos < 0) return NULL; - lb_val->last_sel_pos = pos; + lb_val->last_sel_pos = (uint16_t)pos; return (uint8_t *)&lb_val->back_end_ips[pos][0]; } diff --git a/src/dp_log.c b/src/dp_log.c index cb1d3dc41..687ab3d24 100644 --- a/src/dp_log.c +++ b/src/dp_log.c @@ -182,8 +182,8 @@ static const char *json_escape(const char *message, char *buf, size_t bufsize) buf[bufpos++] = 'u'; buf[bufpos++] = '0'; buf[bufpos++] = '0'; - buf[bufpos++] = '0' + hi; - buf[bufpos++] = lo >= 10 ? 'a' + (lo-10) : '0' + lo; + buf[bufpos++] = (char)('0' + hi); + buf[bufpos++] = (char)(lo >= 10 ? 'a' + (lo-10) : '0' + lo); } else if (c == '\\' || c == '\"') { if (bufpos + 2 >= bufsize) break; diff --git a/src/dp_lpm.c b/src/dp_lpm.c index f18cd4253..24733c4f5 100644 --- a/src/dp_lpm.c +++ b/src/dp_lpm.c @@ -159,7 +159,7 @@ static int dp_list_route_entry(struct rte_rib_node *node, // can only fail when any argument is NULL rte_rib_get_nh(node, &next_hop); - dst_port = dp_get_port_by_id(next_hop); + dst_port = dp_get_port_by_id((uint16_t)next_hop); if (unlikely(!dst_port)) return DP_GRPC_ERR_NO_VM; @@ -297,7 +297,7 @@ const struct dp_port *dp_get_ip4_out_port(const struct dp_port *in_port, if (DP_FAILED(rte_rib_get_nh(node, &next_hop))) return NULL; - dst_port = dp_get_port_by_id(next_hop); + dst_port = dp_get_port_by_id((uint16_t)next_hop); if (!dst_port) return NULL; @@ -334,7 +334,7 @@ const struct dp_port *dp_get_ip6_out_port(const struct dp_port *in_port, if (DP_FAILED(rte_rib6_get_nh(node, &next_hop))) return NULL; - dst_port = dp_get_port_by_id(next_hop); + dst_port = dp_get_port_by_id((uint16_t)next_hop); if (!dst_port) return NULL; diff --git a/src/dp_netlink.c b/src/dp_netlink.c index 8189c3d54..3a819c5ea 100644 --- a/src/dp_netlink.c +++ b/src/dp_netlink.c @@ -16,7 +16,7 @@ static int dp_read_neigh(struct nlmsghdr *nh, __u32 nll, struct rte_ether_addr * { struct rtattr *rt_attr; struct ndmsg *rt_msg; - int rtl, ndm_family; + size_t rtl, ndm_family; for (; NLMSG_OK(nh, nll); nh = NLMSG_NEXT(nh, nll)) { rt_msg = (struct ndmsg *)NLMSG_DATA(nh); @@ -40,13 +40,13 @@ static int dp_read_neigh(struct nlmsghdr *nh, __u32 nll, struct rte_ether_addr * static int dp_recv_msg(struct sockaddr_nl sock_addr, int sock, char *buf, int bufsize) { struct nlmsghdr *nh; - int recv_len; - int msg_len = 0; + ssize_t recv_len; + ssize_t msg_len = 0; for (;;) { recv_len = recv(sock, buf, bufsize - msg_len, 0); if (recv_len < 0) - return recv_len; + return (int)recv_len; nh = (struct nlmsghdr *)buf; if (nh->nlmsg_type == NLMSG_DONE) @@ -60,7 +60,7 @@ static int dp_recv_msg(struct sockaddr_nl sock_addr, int sock, char *buf, int bu if ((sock_addr.nl_groups & RTMGRP_IPV6_ROUTE) == RTMGRP_IPV6_ROUTE) break; } - return msg_len; + return (int)msg_len; } int dp_get_pf_neigh_mac(int if_idx, struct rte_ether_addr *neigh, const struct rte_ether_addr *own_mac) diff --git a/src/dp_port.c b/src/dp_port.c index d70740f9d..c9453fd0c 100644 --- a/src/dp_port.c +++ b/src/dp_port.c @@ -88,7 +88,7 @@ static int dp_port_init_ethdev(struct dp_port *port, struct rte_eth_dev_info *de port_conf.txmode.offloads &= dev_info->tx_offload_capa; nr_hairpin_queues = port->is_pf - ? (DP_NR_PF_HAIRPIN_RX_TX_QUEUES + DP_NR_VF_HAIRPIN_RX_TX_QUEUES * dp_layer->num_of_vfs) + ? (uint16_t)(DP_NR_PF_HAIRPIN_RX_TX_QUEUES + DP_NR_VF_HAIRPIN_RX_TX_QUEUES * dp_layer->num_of_vfs) : DP_NR_VF_HAIRPIN_RX_TX_QUEUES; ret = rte_eth_dev_configure(port->port_id, @@ -104,7 +104,7 @@ static int dp_port_init_ethdev(struct dp_port *port, struct rte_eth_dev_info *de rxq_conf.offloads = port_conf.rxmode.offloads; /* RX and TX queues config */ - for (int i = 0; i < DP_NR_STD_RX_QUEUES; ++i) { + for (uint16_t i = 0; i < DP_NR_STD_RX_QUEUES; ++i) { ret = rte_eth_rx_queue_setup(port->port_id, i, 1024, port->socket_id, &rxq_conf, @@ -118,7 +118,7 @@ static int dp_port_init_ethdev(struct dp_port *port, struct rte_eth_dev_info *de txq_conf = dev_info->default_txconf; txq_conf.offloads = port_conf.txmode.offloads; - for (int i = 0; i < DP_NR_STD_TX_QUEUES; ++i) { + for (uint16_t i = 0; i < DP_NR_STD_TX_QUEUES; ++i) { ret = rte_eth_tx_queue_setup(port->port_id, i, 2048, port->socket_id, &txq_conf); @@ -223,7 +223,7 @@ static struct dp_port *dp_port_init_interface(uint16_t port_id, struct rte_eth_d // All VFs belong to pf0, assign a tx queue from pf1 for it if (dp_conf_is_offload_enabled()) { port->peer_pf_port_id = dp_get_pf1()->port_id; - port->peer_pf_hairpin_tx_rx_queue_offset = last_pf1_hairpin_tx_rx_queue_offset++; + port->peer_pf_hairpin_tx_rx_queue_offset = (uint8_t)last_pf1_hairpin_tx_rx_queue_offset++; if (last_pf1_hairpin_tx_rx_queue_offset > UINT8_MAX) { DPS_LOG_ERR("Too many VFs, cannot create more hairpins"); return NULL; @@ -357,7 +357,7 @@ void dp_ports_free(void) } -static int dp_port_install_isolated_mode(int port_id) +static int dp_port_install_isolated_mode(uint16_t port_id) { DPS_LOG_INFO("Init isolation flow rule for IPinIP tunnels"); if (DP_FAILED(dp_install_isolated_mode_ipip(port_id, DP_IP_PROTO_IPv4_ENCAP)) diff --git a/src/dp_service.c b/src/dp_service.c index 7f138465e..83c45817d 100644 --- a/src/dp_service.c +++ b/src/dp_service.c @@ -216,7 +216,8 @@ static int run_service(void) { int result; - srand(rte_rdtsc()); + // the lower 32 bits are the ones that are actually changing all the time, cast is fine + srand((unsigned int)rte_rdtsc()); // pre-init sanity checks if (!dp_conf_is_conntrack_enabled() && dp_conf_is_offload_enabled()) { diff --git a/src/dp_telemetry.c b/src/dp_telemetry.c index f9efc16fe..1011da612 100644 --- a/src/dp_telemetry.c +++ b/src/dp_telemetry.c @@ -43,7 +43,7 @@ static int tel_stat_value_offset = 0; // // Graph introduces another layer of callbacks due to DPDK stats API // -static __rte_always_inline int get_stat_value(const struct rte_graph_cluster_node_stats *st) +static __rte_always_inline uint64_t get_stat_value(const struct rte_graph_cluster_node_stats *st) { return *(const uint64_t *)((const uint8_t *)(st) + tel_stat_value_offset); } diff --git a/src/dp_util.c b/src/dp_util.c index 37a148476..743641098 100644 --- a/src/dp_util.c +++ b/src/dp_util.c @@ -12,7 +12,7 @@ #define DP_SYSFS_MAX_PATH 256 // makes sure there is enough space to prevent collisions -#define DP_JHASH_MARGIN_COEF(ENTRIES) ((ENTRIES)*1.20) +#define DP_JHASH_MARGIN_COEF(ENTRIES) ((uint32_t)((ENTRIES)*1.20)) int dp_get_dev_info(uint16_t port_id, struct rte_eth_dev_info *dev_info, char ifname[IF_NAMESIZE]) { @@ -138,7 +138,7 @@ struct rte_hash *dp_create_jhash_table(int entries, size_t key_len, const char * struct rte_hash_parameters params = { .name = full_name, .entries = DP_JHASH_MARGIN_COEF(entries), - .key_len = key_len, + .key_len = (uint32_t)key_len, // no way this will get bigger than 32b .hash_func = hash_func, .hash_func_init_val = 0xfee1900d, // "random" IV .socket_id = socket_id, diff --git a/src/dp_virtsvc.c b/src/dp_virtsvc.c index fb1a7d4dc..8105a8ab2 100644 --- a/src/dp_virtsvc.c +++ b/src/dp_virtsvc.c @@ -214,7 +214,8 @@ void dp_virtsvc_free(void) int dp_virtsvc_get_count(void) { - return dp_virtservices_end - dp_virtservices; + // conversion is fine, will never get that far + return (int)(dp_virtservices_end - dp_virtservices); } @@ -255,11 +256,11 @@ static __rte_always_inline int dp_virtsvc_get_free_port(struct dp_virtsvc *virts virtsvc->last_assigned_port = 0; #endif - for (int port = virtsvc->last_assigned_port+1; port < DP_VIRTSVC_PORTCOUNT; ++port) + for (uint16_t port = virtsvc->last_assigned_port+1; port < DP_VIRTSVC_PORTCOUNT; ++port) if (dp_virtsvc_is_connection_old(&virtsvc->connections[port], current_tsc)) return virtsvc->last_assigned_port = port; - for (int port = 0; port <= virtsvc->last_assigned_port; ++port) + for (uint16_t port = 0; port <= virtsvc->last_assigned_port; ++port) if (dp_virtsvc_is_connection_old(&virtsvc->connections[port], current_tsc)) return virtsvc->last_assigned_port = port; @@ -317,7 +318,7 @@ static __rte_always_inline int dp_virstvc_get_connection(struct dp_virtsvc *virt return ret; assert((intptr_t)data >= 0 && (intptr_t)data < DP_VIRTSVC_PORTCOUNT); - return (intptr_t)data; + return (int)(intptr_t)data; } int dp_virtsvc_get_pf_route(struct dp_virtsvc *virtsvc, diff --git a/src/dp_vnf.c b/src/dp_vnf.c index 86148205e..6cd3f7c55 100644 --- a/src/dp_vnf.c +++ b/src/dp_vnf.c @@ -25,7 +25,7 @@ void dp_vnf_free(void) } int dp_add_vnf(const uint8_t ul_addr6[DP_VNF_IPV6_ADDR_SIZE], enum dp_vnf_type type, - uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint16_t prefix_len) + uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint8_t prefix_len) { struct dp_vnf *vnf; int ret; @@ -91,7 +91,7 @@ int dp_del_vnf(const uint8_t ul_addr6[DP_VNF_IPV6_ADDR_SIZE]) static __rte_always_inline bool dp_vnf_match(const struct dp_vnf *vnf, enum dp_vnf_type type, uint16_t port_id, uint32_t vni, - uint32_t prefix_ip, uint16_t prefix_len) + uint32_t prefix_ip, uint8_t prefix_len) { return (port_id == DP_VNF_MATCH_ALL_PORT_IDS || vnf->port_id == port_id) && vnf->vni == vni @@ -100,7 +100,7 @@ static __rte_always_inline bool dp_vnf_match(const struct dp_vnf *vnf, enum dp_v && vnf->type == type; } -bool dp_vnf_lbprefix_exists(uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint16_t prefix_len) +bool dp_vnf_lbprefix_exists(uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint8_t prefix_len) { struct dp_vnf *vnf; uint32_t iter = 0; @@ -119,7 +119,7 @@ bool dp_vnf_lbprefix_exists(uint16_t port_id, uint32_t vni, uint32_t prefix_ip, return false; } -int dp_del_vnf_by_value(enum dp_vnf_type type, uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint16_t prefix_len) +int dp_del_vnf_by_value(enum dp_vnf_type type, uint16_t port_id, uint32_t vni, uint32_t prefix_ip, uint8_t prefix_len) { struct dp_vnf *vnf; uint32_t iter = 0; diff --git a/src/dpdk_layer.c b/src/dpdk_layer.c index 828b11962..42d1cce37 100644 --- a/src/dpdk_layer.c +++ b/src/dpdk_layer.c @@ -119,7 +119,8 @@ static int main_core_loop(void) uint64_t prev_cycles = 0; uint64_t elapsed_cycles; uint64_t period_cycles = dp_timers_get_manage_interval_cycles(); - double cycles_per_ns = rte_get_timer_hz() / (double)NS_PER_S; + uint64_t timer_hz = rte_get_timer_hz(); + double cycles_per_ns = (double)timer_hz / (double)NS_PER_S; int ret = DP_OK; while (!force_quit) { @@ -128,7 +129,7 @@ static int main_core_loop(void) if (elapsed_cycles < period_cycles) { // rte_delay_us_sleep() is not interruptible by signals // (and signal is something that should stop this loop) - dp_nanosleep((period_cycles - elapsed_cycles) / cycles_per_ns); + dp_nanosleep((uint64_t)((double)(period_cycles - elapsed_cycles) / cycles_per_ns)); // if wait fails, this effectively becomes busy-wait, which is fine continue; } diff --git a/src/grpc/dp_async_grpc.cpp b/src/grpc/dp_async_grpc.cpp index 0cc55e9ac..5d5cb7a00 100644 --- a/src/grpc/dp_async_grpc.cpp +++ b/src/grpc/dp_async_grpc.cpp @@ -27,7 +27,7 @@ CallState BaseCall::HandleRpc() switch (call_state_) { case REQUEST: { struct dpgrpc_request request = { - .type = (uint16_t)call_type_, + .type = call_type_, }; const char* error; // this is how gRPC is implemented here, @@ -314,7 +314,9 @@ const char* CreatePrefixCall::FillRequest(struct dpgrpc_request* request) return "Invalid interface_id"; if (!GrpcConv::GrpcToDpAddress(request_.prefix().ip(), &request->del_pfx.addr)) return "Invalid prefix.ip"; - request->del_pfx.length = request_.prefix().length(); + if (request_.prefix().length() > UINT8_MAX) + return "Invalid prefix.length"; + request->del_pfx.length = (uint8_t)request_.prefix().length(); return NULL; } void CreatePrefixCall::ParseReply(struct dpgrpc_reply* reply) @@ -335,7 +337,9 @@ const char* DeletePrefixCall::FillRequest(struct dpgrpc_request* request) return "Invalid interface_id"; if (!GrpcConv::GrpcToDpAddress(request_.prefix().ip(), &request->del_pfx.addr)) return "Invalid prefix.ip"; - request->del_pfx.length = request_.prefix().length(); + if (request_.prefix().length() > UINT8_MAX) + return "Invalid prefix.length"; + request->del_pfx.length = (uint8_t)request_.prefix().length(); return NULL; } void DeletePrefixCall::ParseReply(__rte_unused struct dpgrpc_reply* reply) @@ -383,7 +387,9 @@ const char* CreateRouteCall::FillRequest(struct dpgrpc_request* request) DP_LOG_IPV6STR(request_.route().nexthop_address().address().c_str())); request->add_route.vni = request_.vni(); request->add_route.trgt_vni = request_.route().nexthop_vni(); - request->add_route.pfx_length = request_.route().prefix().length(); + if (request_.route().prefix().length() > UINT8_MAX) + return "Invalid route.prefix.length"; + request->add_route.pfx_length = (uint8_t)request_.route().prefix().length(); if (!GrpcConv::GrpcToDpAddress(request_.route().prefix().ip(), &request->add_route.pfx_addr)) return "Invalid route.prefix.ip"; if (!GrpcConv::GrpcToDpAddress(request_.route().nexthop_address(), &request->add_route.trgt_addr)) @@ -401,7 +407,9 @@ const char* DeleteRouteCall::FillRequest(struct dpgrpc_request* request) DP_LOG_PREFIX(request_.route().prefix().ip().address().c_str()), DP_LOG_PREFLEN(request_.route().prefix().length())); request->del_route.vni = request_.vni(); - request->add_route.pfx_length = request_.route().prefix().length(); + if (request_.route().prefix().length() > UINT8_MAX) + return "Invalid route.prefix.length"; + request->add_route.pfx_length = (uint8_t)request_.route().prefix().length(); if (!GrpcConv::GrpcToDpAddress(request_.route().prefix().ip(), &request->add_route.pfx_addr)) return "Invalid route.prefix.ip"; return NULL; @@ -529,8 +537,8 @@ const char* CreateNatCall::FillRequest(struct dpgrpc_request* request) return "Invalid max_port"; if (request_.min_port() >= request_.max_port()) return "Invalid port range"; - request->add_nat.min_port = request_.min_port(); - request->add_nat.max_port = request_.max_port(); + request->add_nat.min_port = (uint16_t)request_.min_port(); + request->add_nat.max_port = (uint16_t)request_.max_port(); return NULL; } void CreateNatCall::ParseReply(struct dpgrpc_reply* reply) @@ -618,8 +626,8 @@ const char* CreateNeighborNatCall::FillRequest(struct dpgrpc_request* request) return "Invalid max_port"; if (request_.min_port() >= request_.max_port()) return "Invalid port range"; - request->add_neighnat.min_port = request_.min_port(); - request->add_neighnat.max_port = request_.max_port(); + request->add_neighnat.min_port = (uint16_t)request_.min_port(); + request->add_neighnat.max_port = (uint16_t)request_.max_port(); request->add_neighnat.vni = request_.vni(); if (!GrpcConv::StrToIpv6(request_.underlay_route(), request->add_neighnat.neigh_addr6)) return "Invalid underlay_route"; @@ -644,8 +652,8 @@ const char* DeleteNeighborNatCall::FillRequest(struct dpgrpc_request* request) return "Invalid max_port"; if (request_.min_port() >= request_.max_port()) return "Invalid port range"; - request->del_neighnat.min_port = request_.min_port(); - request->del_neighnat.max_port = request_.max_port(); + request->del_neighnat.min_port = (uint16_t)request_.min_port(); + request->del_neighnat.max_port = (uint16_t)request_.max_port(); request->del_neighnat.vni = request_.vni(); // neigh_addr6 field is implied by this unique NAT definition return NULL; @@ -699,7 +707,7 @@ const char* CreateLoadBalancerCall::FillRequest(struct dpgrpc_request* request) DP_LOG_PROTO(request_.loadbalanced_ports(i).protocol())); if (request_.loadbalanced_ports(i).port() > UINT16_MAX) return "Invalid loadbalanced_ports.port"; - request->add_lb.lbports[i].port = request_.loadbalanced_ports(i).port(); + request->add_lb.lbports[i].port = (uint16_t)request_.loadbalanced_ports(i).port(); if (request_.loadbalanced_ports(i).protocol() == TCP) request->add_lb.lbports[i].protocol = DP_IP_PROTO_TCP; else if (request_.loadbalanced_ports(i).protocol() == UDP) @@ -829,7 +837,9 @@ const char* CreateLoadBalancerPrefixCall::FillRequest(struct dpgrpc_request* req return "Invalid interface_id"; if (!GrpcConv::GrpcToDpAddress(request_.prefix().ip(), &request->add_lbpfx.addr)) return "Invalid prefix.ip"; - request->add_lbpfx.length = request_.prefix().length(); + if (request_.prefix().length() > UINT8_MAX) + return "Invalid prefix.length"; + request->add_lbpfx.length = (uint8_t)request_.prefix().length(); return NULL; } void CreateLoadBalancerPrefixCall::ParseReply(struct dpgrpc_reply* reply) @@ -850,7 +860,9 @@ const char* DeleteLoadBalancerPrefixCall::FillRequest(struct dpgrpc_request* req return "Invalid interface_id"; if (!GrpcConv::GrpcToDpAddress(request_.prefix().ip(), &request->del_lbpfx.addr)) return "Invalid prefix.ip"; - request->del_lbpfx.length = request_.prefix().length(); + if (request_.prefix().length() > UINT8_MAX) + return "Invalid prefix.length"; + request->del_lbpfx.length = (uint8_t)request_.prefix().length(); return NULL; } void DeleteLoadBalancerPrefixCall::ParseReply(__rte_unused struct dpgrpc_reply* reply) @@ -929,7 +941,7 @@ const char* CreateFirewallRuleCall::FillRequest(struct dpgrpc_request* request) return "Invalid action"; if (grpc_rule.priority() > UINT16_MAX) return "Invalid priority"; - dp_rule->priority = grpc_rule.priority(); + dp_rule->priority = (uint16_t)grpc_rule.priority(); switch (grpc_filter.filter_case()) { case ProtocolFilter::kTcpFieldNumber: @@ -1102,8 +1114,8 @@ const char* CaptureStartCall::FillRequest(struct dpgrpc_request* request) if (!GrpcConv::StrToIpv6(request_.capture_config().sink_node_ip().address(), request->capture_start.dst_addr6)) return "Invalid sink_node_ip"; - request->capture_start.udp_src_port = request_.capture_config().udp_src_port(); - request->capture_start.udp_dst_port = request_.capture_config().udp_dst_port(); + request->capture_start.udp_src_port = (uint16_t)request_.capture_config().udp_src_port(); + request->capture_start.udp_dst_port = (uint16_t)request_.capture_config().udp_dst_port(); if (request_.capture_config().interfaces_size() > DP_CAPTURE_MAX_PORT_NUM) return "Too many interfaces to be captured"; @@ -1115,17 +1127,19 @@ const char* CaptureStartCall::FillRequest(struct dpgrpc_request* request) switch (request->capture_start.interfaces[i].type) { case DP_CAPTURE_IFACE_TYPE_SINGLE_VF: - DPGRPC_LOG_INFO("Set packet capture interface vf", + DPGRPC_LOG_INFO("Setting packet capture on VF", DP_LOG_IFACE_TYPE(request_.capture_config().interfaces(i).interface_type()), DP_LOG_IFACE(request_.capture_config().interfaces(i).vf_name().c_str())); if (SNPRINTF_FAILED(request->capture_start.interfaces[i].spec.iface_id, request_.capture_config().interfaces(i).vf_name())) return "Invalid interface_id"; break; case DP_CAPTURE_IFACE_TYPE_SINGLE_PF: - DPGRPC_LOG_INFO("Set packet capture interface pf", + DPGRPC_LOG_INFO("Setting packet capture on PF", DP_LOG_IFACE_TYPE(request_.capture_config().interfaces(i).interface_type()), DP_LOG_IFACE_INDEX(request_.capture_config().interfaces(i).pf_index())); - request->capture_start.interfaces[i].spec.pf_index = request_.capture_config().interfaces(i).pf_index(); + if (request_.capture_config().interfaces(i).pf_index() > UINT8_MAX) + return "Invalid pf_index"; + request->capture_start.interfaces[i].spec.pf_index = (uint8_t)request_.capture_config().interfaces(i).pf_index(); break; } diff --git a/src/grpc/dp_grpc_impl.c b/src/grpc/dp_grpc_impl.c index 4dc9dbb94..9c05539c8 100644 --- a/src/grpc/dp_grpc_impl.c +++ b/src/grpc/dp_grpc_impl.c @@ -35,7 +35,7 @@ static __rte_always_inline void dp_generate_underlay_ipv6(uint8_t route[DP_VNF_I #ifdef ENABLE_STATIC_UNDERLAY_IP random_byte = 1; #else - random_byte = rand() % 256; + random_byte = (uint8_t)(rand() % 256); #endif /* 1 byte random value */ @@ -56,7 +56,7 @@ static __rte_always_inline void dp_generate_underlay_ipv6(uint8_t route[DP_VNF_I static int dp_create_vnf_route(uint8_t ul_addr6[DP_VNF_IPV6_ADDR_SIZE] /* out */, enum dp_vnf_type type, uint32_t vni, const struct dp_port *port, - uint32_t prefix_ip, uint16_t prefix_len) + uint32_t prefix_ip, uint8_t prefix_len) { dp_generate_underlay_ipv6(ul_addr6); return dp_add_vnf(ul_addr6, type, port->port_id, vni, prefix_ip, prefix_len); @@ -891,7 +891,7 @@ static int dp_process_capture_stop(struct dp_grpc_responder *responder) return ret; } - reply->port_cnt = ret; + reply->port_cnt = (uint16_t)ret; dp_set_capture_enabled(false); return DP_GRPC_OK; } @@ -901,7 +901,7 @@ static int dp_process_capture_status(struct dp_grpc_responder *responder) struct dpgrpc_capture *reply = dp_grpc_single_reply(responder); const struct dp_ports *ports = dp_get_ports(); const struct dp_capture_hdr_config *capture_hdr_config = dp_get_capture_hdr_config(); - int count = 0; + uint8_t count = 0; if (!dp_is_capture_enabled()) { memset(reply, 0, sizeof(*reply)); @@ -914,6 +914,8 @@ static int dp_process_capture_status(struct dp_grpc_responder *responder) continue; // this should never happen, but just in case + static_assert(DP_CAPTURE_MAX_PORT_NUM < UINT8_MAX, + "Possible number of captured ports is too big"); if (count >= DP_CAPTURE_MAX_PORT_NUM) { DPS_LOG_ERR("Unexpected number of interfaces are being captured", DP_LOG_VALUE(count), DP_LOG_MAX(DP_CAPTURE_MAX_PORT_NUM)); @@ -945,7 +947,7 @@ static int dp_process_capture_status(struct dp_grpc_responder *responder) void dp_process_request(struct rte_mbuf *m) { struct dp_grpc_responder responder; - uint8_t request_type; + enum dpgrpc_request_type request_type; int ret; request_type = dp_grpc_init_responder(&responder, m); diff --git a/src/grpc/dp_grpc_responder.c b/src/grpc/dp_grpc_responder.c index 9ee897fb9..3cd083b37 100644 --- a/src/grpc/dp_grpc_responder.c +++ b/src/grpc/dp_grpc_responder.c @@ -1,7 +1,7 @@ #include "grpc/dp_grpc_responder.h" #include "dp_log.h" -uint16_t dp_grpc_init_responder(struct dp_grpc_responder *responder, struct rte_mbuf *req_mbuf) +enum dpgrpc_request_type dp_grpc_init_responder(struct dp_grpc_responder *responder, struct rte_mbuf *req_mbuf) { void *req_payload = rte_pktmbuf_mtod(req_mbuf, void *); @@ -31,6 +31,11 @@ int dp_grpc_alloc_reply(struct dp_grpc_responder *responder) struct rte_mbuf *m_new; struct dpgrpc_reply *rep_new; + if (responder->rep_capacity > UINT16_MAX) { + DPGRPC_LOG_WARNING("gRPC response array is too big, truncated", DP_LOG_GRPCREQUEST(responder->request.type)); + return DP_ERROR; + } + if (responder->repcount >= RTE_DIM(responder->replies)) { DPGRPC_LOG_WARNING("gRPC response array is full, truncated", DP_LOG_GRPCREQUEST(responder->request.type)); return DP_ERROR; @@ -42,7 +47,7 @@ int dp_grpc_alloc_reply(struct dp_grpc_responder *responder) } // more reply packets follow this one; as gRPC has only one thread, this is safe to do responder->rep->is_chained = 1; - responder->rep->msg_count = responder->rep_capacity; + responder->rep->msg_count = (uint16_t)responder->rep_capacity; rep_new = rte_pktmbuf_mtod(m_new, struct dpgrpc_reply *); rep_new->type = responder->request.type; rep_new->is_chained = 0; diff --git a/src/monitoring/dp_monitoring.c b/src/monitoring/dp_monitoring.c index 01cdcb186..24a61c0c6 100644 --- a/src/monitoring/dp_monitoring.c +++ b/src/monitoring/dp_monitoring.c @@ -23,7 +23,7 @@ void dp_process_event_msg(struct rte_mbuf *m) rte_pktmbuf_free(m); } -void dp_set_capture_hdr_config(uint8_t *addr, uint32_t udp_src_port, uint32_t udp_dst_port) +void dp_set_capture_hdr_config(uint8_t *addr, uint16_t udp_src_port, uint16_t udp_dst_port) { rte_memcpy(capture_hdr_config.capture_node_ipv6_addr, addr, sizeof(capture_hdr_config.capture_node_ipv6_addr)); capture_hdr_config.capture_udp_src_port = udp_src_port; diff --git a/src/monitoring/dp_pcap.c b/src/monitoring/dp_pcap.c index 603aa0c93..fdf42c3ce 100644 --- a/src/monitoring/dp_pcap.c +++ b/src/monitoring/dp_pcap.c @@ -46,7 +46,7 @@ void dp_pcap_dump(struct dp_pcap *dp_pcap, struct rte_mbuf *m, struct timeval *t header.ts = *timestamp; header.len = rte_pktmbuf_pkt_len(m); - header.caplen = RTE_MIN(header.len, sizeof(buffer)); + header.caplen = RTE_MIN(header.len, (uint32_t)sizeof(buffer)); // rte_pktmbuf_read() can only fail if the buffer is too small, hence RTE_MIN() above pcap_dump((void *)dp_pcap->dumper, &header, rte_pktmbuf_read(m, 0, header.caplen, buffer)); @@ -82,7 +82,7 @@ bool dp_is_bpf_match(struct bpf_program *bpf, struct rte_mbuf *m) uint32_t len = rte_pktmbuf_pkt_len(m); struct pcap_pkthdr header = { .len = len, - .caplen = RTE_MIN(len, sizeof(buffer)), + .caplen = RTE_MIN(len, (uint32_t)sizeof(buffer)), // timestamp left zero, not needed for filtering }; diff --git a/src/nodes/cls_node.c b/src/nodes/cls_node.c index 816b20d0b..931aa2a0c 100644 --- a/src/nodes/cls_node.c +++ b/src/nodes/cls_node.c @@ -67,7 +67,7 @@ static __rte_always_inline struct dp_virtsvc *get_outgoing_virtsvc(const struct { const struct rte_ipv4_hdr *ipv4_hdr = (const struct rte_ipv4_hdr *)(ether_hdr + 1); rte_be32_t addr = ipv4_hdr->dst_addr; - uint16_t proto = ipv4_hdr->next_proto_id; + uint8_t proto = ipv4_hdr->next_proto_id; rte_be16_t port; const struct dp_virtsvc_lookup_entry *entry; int diff; @@ -93,7 +93,7 @@ static __rte_always_inline struct dp_virtsvc *get_outgoing_virtsvc(const struct static __rte_always_inline struct dp_virtsvc *get_incoming_virtsvc(const struct rte_ipv6_hdr *ipv6_hdr) { const uint8_t *addr = ipv6_hdr->src_addr; - uint16_t proto = ipv6_hdr->proto; + uint8_t proto = ipv6_hdr->proto; rte_be16_t port; const struct dp_virtsvc_lookup_entry *entry; int diff; diff --git a/src/nodes/dhcp_node.c b/src/nodes/dhcp_node.c index dcef0ad2e..b8f3df1be 100644 --- a/src/nodes/dhcp_node.c +++ b/src/nodes/dhcp_node.c @@ -50,7 +50,7 @@ static_assert(DP_VIRTSVC_MAX <= UINT8_MAX+1, "Number of virtual services can be static int dhcp_node_init(__rte_unused const struct rte_graph *graph, __rte_unused struct rte_node *node) { server_ip = htonl(dp_get_gw_ip4()); - iface_mtu = htons(dp_conf_get_dhcp_mtu()); + iface_mtu = htons((uint16_t)dp_conf_get_dhcp_mtu()); dhcp_hdr_magic = htonl(DHCP_MAGIC_COOKIE); @@ -111,7 +111,7 @@ static __rte_always_inline int add_dhcp_options(struct dp_dhcp_header *dhcp_hdr, return DP_ERROR; *pos++ = DHCP_OPT_END; - return pos - dhcp_hdr->options; + return (int)(pos - dhcp_hdr->options); } static int parse_options(struct dp_dhcp_header *dhcp_pkt, @@ -183,8 +183,11 @@ static __rte_always_inline rte_edge_t get_next_index(struct rte_node *node, stru incoming_udp_hdr = (struct rte_udp_hdr *)(incoming_ipv4_hdr + 1); dhcp_hdr = (struct dp_dhcp_header *)(incoming_udp_hdr + 1); options_len = rte_pktmbuf_data_len(m) - - ((uint8_t *)dhcp_hdr - (uint8_t *)incoming_eth_hdr) - - offsetof(struct dp_dhcp_header, options); + - (int)((uint8_t *)dhcp_hdr - (uint8_t *)incoming_eth_hdr) + - (int)offsetof(struct dp_dhcp_header, options); + + if (options_len < 0) + return DHCP_NEXT_DROP; if (dhcp_hdr->op != DP_BOOTP_REQUEST) { DPNODE_LOG_WARNING(node, "Not a DHCP request", DP_LOG_VALUE(dhcp_hdr->op)); @@ -264,17 +267,20 @@ static __rte_always_inline rte_edge_t get_next_index(struct rte_node *node, stru // packet length changed because of new options, recompute envelopes header_size = offsetof(struct dp_dhcp_header, options) + options_len; + m->pkt_len = (uint32_t)(sizeof(struct rte_ether_hdr) + + sizeof(struct rte_ipv4_hdr) + + sizeof(struct rte_udp_hdr) + + header_size); + if (unlikely(m->pkt_len > UINT16_MAX)) + return DHCP_NEXT_DROP; + incoming_ipv4_hdr->hdr_checksum = 0; - incoming_ipv4_hdr->total_length = htons(sizeof(struct rte_ipv4_hdr) + incoming_ipv4_hdr->total_length = htons((uint16_t)(sizeof(struct rte_ipv4_hdr) + sizeof(struct rte_udp_hdr) - + header_size); - incoming_udp_hdr->dgram_len = htons(sizeof(struct rte_udp_hdr) + header_size); + + header_size)); + incoming_udp_hdr->dgram_len = htons((uint16_t)(sizeof(struct rte_udp_hdr) + header_size)); incoming_udp_hdr->dgram_cksum = 0; - m->pkt_len = sizeof(struct rte_ether_hdr) - + sizeof(struct rte_ipv4_hdr) - + sizeof(struct rte_udp_hdr) - + header_size; - m->data_len = m->pkt_len; + m->data_len = (uint16_t)m->pkt_len; return next_tx_index[m->port]; } diff --git a/src/nodes/dhcpv6_node.c b/src/nodes/dhcpv6_node.c index 5395825be..7124871f4 100644 --- a/src/nodes/dhcpv6_node.c +++ b/src/nodes/dhcpv6_node.c @@ -71,14 +71,14 @@ static int dhcpv6_node_init(__rte_unused const struct rte_graph *graph, __rte_un static __rte_always_inline int parse_options(struct rte_mbuf *m, const uint8_t *options, - int options_len, + size_t options_len, struct dp_dhcpv6_reply_options *reply_options) { uint16_t op_code; uint16_t op_len = 0; const struct dhcpv6_option *opt; - for (int i = 0; + for (size_t i = 0; i + sizeof(struct dhcpv6_option) < (size_t)options_len; // len already checked for being positive i += sizeof(struct dhcpv6_option) + op_len ) { @@ -130,12 +130,12 @@ static __rte_always_inline int parse_options(struct rte_mbuf *m, static __rte_always_inline int resize_packet(struct rte_mbuf *m, int delta) { if (delta > 0) { - if (!rte_pktmbuf_append(m, delta)) { + if (delta > UINT16_MAX || !rte_pktmbuf_append(m, (uint16_t)delta)) { DPS_LOG_WARNING("Not enough space for DHCPv6 options in packet"); return DP_ERROR; } } else if (delta < 0) { - if (DP_FAILED(rte_pktmbuf_trim(m, -delta))) { + if (delta < -UINT16_MAX || DP_FAILED(rte_pktmbuf_trim(m, (uint16_t)(-delta)))) { DPS_LOG_WARNING("Invalid trim of DHCPv6 packet", DP_LOG_VALUE(-delta)); return DP_ERROR; } @@ -153,7 +153,7 @@ static __rte_always_inline int strip_options(struct rte_mbuf *m, int options_len /** @return size of generated options or error */ static int generate_reply_options(struct rte_mbuf *m, uint8_t *options, int options_len) { - uint8_t reply_options_len; + int reply_options_len; struct dhcpv6_opt_server_id_ll opt_sid; struct dp_dhcpv6_reply_options reply_options = {0}; // this makes *_len fields 0, needed later @@ -165,7 +165,7 @@ static int generate_reply_options(struct rte_mbuf *m, uint8_t *options, int opti opt_sid = opt_sid_template; rte_ether_addr_copy(&rte_pktmbuf_mtod(m, struct rte_ether_hdr *)->dst_addr, &opt_sid.id.mac); - reply_options_len = sizeof(opt_sid) + reply_options.opt_cid_len + reply_options.opt_iana_len + reply_options.opt_rapid_len; + reply_options_len = (int)sizeof(opt_sid) + reply_options.opt_cid_len + reply_options.opt_iana_len + reply_options.opt_rapid_len; if (DP_FAILED(resize_packet(m, reply_options_len - options_len))) return DP_ERROR; @@ -193,8 +193,9 @@ static __rte_always_inline rte_edge_t get_next_index(struct rte_node *node, stru struct rte_ipv6_hdr *req_ipv6_hdr; struct rte_udp_hdr *req_udp_hdr; struct dhcpv6_packet *dhcp_pkt; - int req_options_len = rte_pktmbuf_data_len(m) - DP_DHCPV6_PKT_FIXED_LEN; + int req_options_len = rte_pktmbuf_data_len(m) - (int)DP_DHCPV6_PKT_FIXED_LEN; int reply_options_len; + size_t payload_len; // packet length is uint16_t, negative value means it's less than the required length if (req_options_len < 0) { @@ -236,8 +237,12 @@ static __rte_always_inline rte_edge_t get_next_index(struct rte_node *node, stru if (DP_FAILED(reply_options_len)) return DHCPV6_NEXT_DROP; + payload_len = reply_options_len + DP_DHCPV6_HDR_FIXED_LEN + sizeof(struct rte_udp_hdr); + if (payload_len > UINT16_MAX) + return DHCPV6_NEXT_DROP; + // recompute checksums (offloaded) - req_ipv6_hdr->payload_len = req_udp_hdr->dgram_len = htons(sizeof(struct rte_udp_hdr) + DP_DHCPV6_HDR_FIXED_LEN + reply_options_len); + req_ipv6_hdr->payload_len = req_udp_hdr->dgram_len = htons((uint16_t)payload_len); req_udp_hdr->dgram_cksum = 0; m->ol_flags |= RTE_MBUF_F_TX_IPV6 | RTE_MBUF_F_TX_UDP_CKSUM; diff --git a/src/nodes/packet_relay_node.c b/src/nodes/packet_relay_node.c index 07697a8e9..277ac4f9f 100644 --- a/src/nodes/packet_relay_node.c +++ b/src/nodes/packet_relay_node.c @@ -29,7 +29,7 @@ static __rte_always_inline rte_edge_t lb_nnat_icmp_reply(struct dp_flow *df, str cksum += RTE_BE16(RTE_IP_ICMP_ECHO_REPLY << 8); cksum = (cksum & 0xffff) + (cksum >> 16); cksum = (cksum & 0xffff) + (cksum >> 16); - icmp_hdr->icmp_cksum = ~cksum; + icmp_hdr->icmp_cksum = (uint16_t)~cksum; temp_ip = ipv4_hdr->dst_addr; ipv4_hdr->dst_addr = ipv4_hdr->src_addr; diff --git a/src/nodes/rx_periodic_node.c b/src/nodes/rx_periodic_node.c index f680c8b63..aa53d3068 100644 --- a/src/nodes/rx_periodic_node.c +++ b/src/nodes/rx_periodic_node.c @@ -40,11 +40,11 @@ static __rte_always_inline void handle_nongraph_queues(void) struct rte_mbuf *mbufs[RTE_MAX(DP_INTERNAL_Q_SIZE, DP_GRPC_Q_SIZE)]; unsigned int count, i; - count = rte_ring_sc_dequeue_burst(monitoring_rx_queue, (void **)mbufs, RTE_DIM(mbufs), NULL); + count = rte_ring_sc_dequeue_burst(monitoring_rx_queue, (void **)mbufs, (unsigned int)RTE_DIM(mbufs), NULL); for (i = 0; i < count; ++i) dp_process_event_msg(mbufs[i]); - count = rte_ring_sc_dequeue_burst(grpc_tx_queue, (void **)mbufs, RTE_DIM(mbufs), NULL); + count = rte_ring_sc_dequeue_burst(grpc_tx_queue, (void **)mbufs, (unsigned int)RTE_DIM(mbufs), NULL); for (i = 0; i < count; ++i) dp_process_request(mbufs[i]); } @@ -59,7 +59,7 @@ static uint16_t rx_periodic_node_process(struct rte_graph *graph, void **objs, uint16_t nb_objs) { - unsigned int n_pkts; + uint16_t n_pkts; RTE_SET_USED(nb_objs); // this is a source node, input data is not present yet @@ -72,10 +72,8 @@ static uint16_t rx_periodic_node_process(struct rte_graph *graph, // these packets do not come from a port, instead they enter the graph from a periodic message // which also implies that this will mostly return 0 - n_pkts = rte_ring_dequeue_burst(periodic_msg_queue, - objs, - RTE_GRAPH_BURST_SIZE, - NULL); + static_assert(RTE_GRAPH_BURST_SIZE < UINT16_MAX, "Graph burst size is too large"); + n_pkts = (uint16_t)rte_ring_dequeue_burst(periodic_msg_queue, objs, RTE_GRAPH_BURST_SIZE, NULL); if (likely(!n_pkts)) return 0; diff --git a/src/nodes/virtsvc_node.c b/src/nodes/virtsvc_node.c index bd62a81c1..b1ea84b5f 100644 --- a/src/nodes/virtsvc_node.c +++ b/src/nodes/virtsvc_node.c @@ -59,7 +59,8 @@ static __rte_always_inline void virtsvc_tcp_state_change(struct dp_virtsvc_conn static __rte_always_inline rte_be16_t virtsvc_get_port_for_conn(int conn_idx) { - return htons(conn_idx + DP_NB_SYSTEM_PORTS); + // conn_idx is limited by the port table size, will fit + return htons((uint16_t)(conn_idx + DP_NB_SYSTEM_PORTS)); } static __rte_always_inline uint16_t virtsvc_request_next(struct rte_node *node, @@ -68,7 +69,7 @@ static __rte_always_inline uint16_t virtsvc_request_next(struct rte_node *node, { struct rte_ether_hdr *ether_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *); struct rte_ipv4_hdr *ipv4_hdr = (struct rte_ipv4_hdr *)(ether_hdr + 1); - rte_be16_t payload_len = htons(ntohs(ipv4_hdr->total_length) - sizeof(struct rte_ipv4_hdr)); + uint16_t hdr_total_len = ntohs(ipv4_hdr->total_length); rte_be32_t original_ip = ipv4_hdr->src_addr; uint8_t proto = ipv4_hdr->next_proto_id; uint8_t ttl = ipv4_hdr->time_to_live; @@ -81,6 +82,9 @@ static __rte_always_inline uint16_t virtsvc_request_next(struct rte_node *node, struct dp_port *pf_port; int conn_idx; + if (hdr_total_len < sizeof(struct rte_ipv4_hdr)) + return VIRTSVC_NEXT_DROP; + // replace IPv4 header with IPv6 header rte_pktmbuf_adj(m, sizeof(struct rte_ether_hdr) + sizeof(struct rte_ipv4_hdr)); ether_hdr = (struct rte_ether_hdr *)rte_pktmbuf_prepend(m, sizeof(struct rte_ether_hdr) + sizeof(struct rte_ipv6_hdr)); @@ -92,7 +96,7 @@ static __rte_always_inline uint16_t virtsvc_request_next(struct rte_node *node, ipv6_hdr = (struct rte_ipv6_hdr *)(ether_hdr + 1); ipv6_hdr->vtc_flow = htonl(DP_IP6_VTC_FLOW); - ipv6_hdr->payload_len = payload_len; + ipv6_hdr->payload_len = htons((uint16_t)(hdr_total_len - sizeof(struct rte_ipv4_hdr))); ipv6_hdr->proto = proto; ipv6_hdr->hop_limits = ttl; rte_memcpy(ipv6_hdr->src_addr, service_ul_ip, sizeof(ipv6_hdr->src_addr)); diff --git a/src/rte_flow/dp_rte_flow_init.c b/src/rte_flow/dp_rte_flow_init.c index e16f052c8..2a239f1df 100644 --- a/src/rte_flow/dp_rte_flow_init.c +++ b/src/rte_flow/dp_rte_flow_init.c @@ -19,7 +19,7 @@ static const struct rte_flow_attr dp_flow_attr_prio_ingress = { }; -int dp_install_isolated_mode_ipip(int port_id, uint8_t proto_id) +int dp_install_isolated_mode_ipip(uint16_t port_id, uint8_t proto_id) { struct rte_flow_item_eth eth_spec; // #1 struct rte_flow_item_ipv6 ipv6_spec; // #2 @@ -46,7 +46,7 @@ int dp_install_isolated_mode_ipip(int port_id, uint8_t proto_id) } #ifdef ENABLE_VIRTSVC -int dp_install_isolated_mode_virtsvc(int port_id, uint8_t proto_id, const uint8_t svc_ipv6[16], rte_be16_t svc_port) +int dp_install_isolated_mode_virtsvc(uint16_t port_id, uint8_t proto_id, const uint8_t svc_ipv6[16], rte_be16_t svc_port) { struct rte_flow_item_eth eth_spec; // #1 struct rte_flow_item_ipv6 ipv6_spec; // #2 diff --git a/src/rte_flow/dp_rte_flow_traffic_forward.c b/src/rte_flow/dp_rte_flow_traffic_forward.c index 2cccf0c5a..e6aef8335 100644 --- a/src/rte_flow/dp_rte_flow_traffic_forward.c +++ b/src/rte_flow/dp_rte_flow_traffic_forward.c @@ -92,7 +92,8 @@ static __rte_always_inline int dp_install_rte_flow_with_age(uint16_t port_id, agectx->cntrack = conntrack; agectx->rte_flow = flow; - agectx->port_id = port_id; + static_assert(DP_MAX_PORTS <= UINT8_MAX, "Too many ports for offloading"); + agectx->port_id = (uint8_t)port_id; dp_ref_inc(&conntrack->ref_count); return DP_OK; } diff --git a/tools/dp_grpc_client.cpp b/tools/dp_grpc_client.cpp index 25e50becc..2d5f3cdbc 100644 --- a/tools/dp_grpc_client.cpp +++ b/tools/dp_grpc_client.cpp @@ -1056,7 +1056,7 @@ class GRPCClient { while (pt != NULL) { if (countp == DP_MAX_LB_PORTS) break; - ports[countp++] = atoi(pt); + ports[countp++] = (uint16_t)atoi(pt); pt = strtok(NULL, ","); }