On Wed, Jan 17, 2024 at 05:07:25PM +0800, Dexia Li via dev wrote:
> For userspace vxlan and geneve tunnel tso, this commit
> fix one packet leak and modify style issues.
>
> Signed-off-by: Dexia Li <[email protected]>
> ---
> lib/dp-packet.c | 6 +++---
> lib/dp-packet.h | 46 +++++++++++++++++++++--------------------
> lib/netdev-dpdk.c | 5 ++---
> lib/netdev-native-tnl.c | 26 +++++++++++------------
> lib/netdev.c | 28 +++++++++++++++++++------
> 5 files changed, 64 insertions(+), 47 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index cb20608d7..e7738c37a 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -548,7 +548,7 @@ dp_packet_compare_offsets(struct dp_packet *b1, struct
> dp_packet *b2,
>
> void
> dp_packet_tnl_outer_ol_send_prepare(struct dp_packet *p,
> - uint64_t flags)
> + uint64_t flags)
> {
> if (dp_packet_hwol_is_outer_ipv4_cksum(p)) {
> if (!(flags & NETDEV_TX_OFFLOAD_OUTER_IP_CKSUM)) {
> @@ -558,7 +558,7 @@ dp_packet_tnl_outer_ol_send_prepare(struct dp_packet *p,
> }
> }
>
> - if (!dp_packet_hwol_is_outer_UDP_cksum(p)) {
> + if (!dp_packet_hwol_is_outer_udp_cksum(p)) {
> return;
> }
>
> @@ -596,7 +596,7 @@ dp_packet_ol_send_prepare(struct dp_packet *p, uint64_t
> flags)
> return;
> }
>
> - if (dp_packet_l4_checksum_good(p) && (!tnl_inner)) {
> + if (dp_packet_l4_checksum_good(p) && !tnl_inner) {
> dp_packet_hwol_reset_tx_l4_csum(p);
> return;
> }
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index cf341f09c..c9ec47c31 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -92,16 +92,16 @@ enum dp_packet_offload_mask {
> /* Offload packet is tunnel VXLAN. */
> DEF_OL_FLAG(DP_PACKET_OL_TX_TUNNEL_VXLAN,
> RTE_MBUF_F_TX_TUNNEL_VXLAN, 0x4000),
> - /* Offload tunnel packet, out is ipv4 */
> + /* Offload tunnel packet, out is IPV4 */
> DEF_OL_FLAG(DP_PACKET_OL_TX_OUTER_IPV4,
> RTE_MBUF_F_TX_OUTER_IPV4, 0x8000),
> - /* Offload TUNNEL out ipv4 checksum */
> + /* Offload tunnel out IPV4 checksum */
> DEF_OL_FLAG(DP_PACKET_OL_TX_OUTER_IP_CKSUM,
> RTE_MBUF_F_TX_OUTER_IP_CKSUM, 0x10000),
> - /* Offload TUNNEL out udp checksum */
> + /* Offload tunnel out UDP checksum */
> DEF_OL_FLAG(DP_PACKET_OL_TX_OUTER_UDP_CKSUM,
> RTE_MBUF_F_TX_OUTER_UDP_CKSUM, 0x20000),
> - /* Offload tunnel packet, out is ipv6 */
> + /* Offload tunnel packet, out is IPV6 */
> DEF_OL_FLAG(DP_PACKET_OL_TX_OUTER_IPV6,
> RTE_MBUF_F_TX_OUTER_IPV6, 0x40000),
>
> @@ -164,9 +164,9 @@ struct dp_packet {
> * or UINT16_MAX. */
> uint16_t l4_ofs; /* Transport-level header offset,
> or UINT16_MAX. */
> - uint16_t inner_l3_ofs; /* inner Network-level header offset,
> + uint16_t inner_l3_ofs; /* Inner Network-level header offset,
> * or UINT16_MAX. */
> - uint16_t inner_l4_ofs; /* inner Transport-level header offset,
> + uint16_t inner_l4_ofs; /* Inner Transport-level header offset,
> or UINT16_MAX. */
> uint32_t cutlen; /* length in bytes to cut from the end. */
> ovs_be32 packet_type; /* Packet type as defined in OpenFlow */
> @@ -279,8 +279,8 @@ bool dp_packet_compare_offsets(struct dp_packet *good,
> struct dp_packet *test,
> struct ds *err_str);
> void dp_packet_ol_send_prepare(struct dp_packet *, uint64_t);
> -void dp_packet_tnl_outer_ol_send_prepare(struct dp_packet *p,
> - uint64_t flags);
> +void dp_packet_tnl_outer_ol_send_prepare(struct dp_packet *,
> + uint64_t);
>
>
>
> @@ -640,19 +640,19 @@ dp_packet_flow_mark_ptr(const struct dp_packet *b)
> static inline void
> dp_packet_set_l2_len(struct dp_packet *b OVS_UNUSED, size_t l2_len
> OVS_UNUSED)
> {
> - /* There are no implementation */
> + /* There is no implementation. */
> }
>
> static inline void
> dp_packet_set_l3_len(struct dp_packet *b OVS_UNUSED, size_t l3_len
> OVS_UNUSED)
> {
> - /* There are no implementation */
> + /* There is no implementation. */
> }
>
> static inline void
> dp_packet_set_l4_len(struct dp_packet *b OVS_UNUSED, size_t l4_len
> OVS_UNUSED)
> {
> - /* There are no implementation */
> + /* There is no implementation. */
> }
>
> static inline uint32_t *
> @@ -1162,23 +1162,26 @@ dp_packet_hwol_is_tunnel_vxlan(struct dp_packet *b)
> return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_TUNNEL_VXLAN);
> }
>
> -/* Returns 'true' if packet 'b' is marked for out ipv4. */
> +/* Returns 'true' if packet 'b' is marked as a tunnel
> + * packet with outer header being IPv4. */
> static inline bool
> dp_packet_hwol_is_outer_ipv4(struct dp_packet *b)
> {
> return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_OUTER_IPV4);
> }
>
> -/* Returns 'true' if packet 'b' is marked for out ipv4 csum offload. */
> +/* Returns 'true' if a tunnel packet 'b' is marked for IPv4 checksum offload
> + * for the outer header. */
> static inline bool
> dp_packet_hwol_is_outer_ipv4_cksum(struct dp_packet *b)
> {
> return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_OUTER_IP_CKSUM);
> }
>
> -/* Returns 'true' if packet 'b' is marked for out udp csum offload. */
> +/* Returns 'true' if a tunnel packet 'b' is marked for UDP checksum offload
> + * for the outer header. */
> static inline bool
> -dp_packet_hwol_is_outer_UDP_cksum(struct dp_packet *b)
> +dp_packet_hwol_is_outer_udp_cksum(struct dp_packet *b)
> {
> return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_OUTER_UDP_CKSUM);
> }
> @@ -1205,7 +1208,8 @@ dp_packet_hwol_set_tx_ipv6(struct dp_packet *a)
> *dp_packet_ol_flags_ptr(a) |= DP_PACKET_OL_TX_IPV6;
> }
>
> -/* Mark packet 'a' as IPv6. */
> +/* Returns 'true' if packet 'b' is marked as a tunnel
> + * packet with outer header being IPv6. */
> static inline void
> dp_packet_hwol_set_tx_outer_ipv6(struct dp_packet *a)
> {
> @@ -1266,30 +1270,28 @@ dp_packet_hwol_set_tcp_seg(struct dp_packet *b)
> *dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_TCP_SEG;
> }
>
> -/* Mark packet 'b' for tunnel geneve offloading. It implies that
> - * the packet 'b' is marked for tunnel geneve offloading. */
> +/* Mark packet 'b' for tunnel geneve offloading. */
> static inline void
> dp_packet_hwol_set_tunnel_geneve(struct dp_packet *b)
> {
> *dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_TUNNEL_GENEVE;
> }
>
> -/* Mark packet 'b' for tunnel vxlan offloading. It implies that
> - * the packet 'b' is marked for tunnel vxlan offloading. */
> +/* Mark packet 'b' for tunnel vxlan offloading. */
> static inline void
> dp_packet_hwol_set_tunnel_vxlan(struct dp_packet *b)
> {
> *dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_TUNNEL_VXLAN;
> }
>
> -/* Mark packet 'b' for out ipv4 packet. */
> +/* Mark packet 'b' as a tunnel packet with IPv4 outer header. */
> static inline void
> dp_packet_hwol_set_tx_outer_ipv4(struct dp_packet *b)
> {
> *dp_packet_ol_flags_ptr(b) |= DP_PACKET_OL_TX_OUTER_IPV4;
> }
>
> -/* Mark packet 'b' for out ipv4 csum offloading. */
> +/* Mark packet 'b' for IPv4 checksum offload for the outer header. */
> static inline void
> dp_packet_hwol_set_tx_outer_ipv4_csum(struct dp_packet *b)
> {
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index e4d95d0a3..fb26825ff 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1157,7 +1157,6 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int
> n_rxq, int n_txq)
> conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_OUTER_UDP_CKSUM;
> }
>
> -
> /* Limit configured rss hash functions to only those supported
> * by the eth device. */
> conf.rx_adv_conf.rss_conf.rss_hf &= info.flow_type_rss_offloads;
> @@ -2570,8 +2569,8 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
> struct rte_mbuf *mbuf)
>
> if (mbuf->ol_flags & (RTE_MBUF_F_TX_TUNNEL_GENEVE |
> RTE_MBUF_F_TX_TUNNEL_VXLAN)) {
> - mbuf->tso_segsz = dev->mtu - mbuf->l2_len - mbuf->l3_len -
> - mbuf->l4_len - mbuf->outer_l3_len;
> + mbuf->tso_segsz = dev->mtu - mbuf->l2_len - mbuf->l3_len -
> + mbuf->l4_len - mbuf->outer_l3_len;
> } else {
> mbuf->l4_len = TCP_OFFSET(th->tcp_ctl) * 4;
> mbuf->tso_segsz = dev->mtu - mbuf->l3_len - mbuf->l4_len;
...
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index 35767800c..d513194dc 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -245,8 +245,8 @@ udp_extract_tnl_md(struct dp_packet *packet, struct
> flow_tnl *tnl,
> * encapsulated now. */
> static void
> dp_packet_tnl_ol_process(const struct netdev *netdev,
> - struct dp_packet *packet,
> - const struct ovs_action_push_tnl *data)
> + struct dp_packet *packet,
> + const struct ovs_action_push_tnl *data)
> {
> struct udp_header *udp = NULL;
> uint8_t opt_len = 0;
...
> @@ -269,10 +269,10 @@ dp_packet_tnl_ol_process(const struct netdev *netdev,
> }
>
> dp_packet_set_l3_len(packet, (char *) dp_packet_l4(packet) -
> - (char *) dp_packet_l3(packet));
> + (char *) dp_packet_l3(packet));
>
> - if (!strcmp(netdev_get_type(netdev), "geneve") ||
> - !strcmp(netdev_get_type(netdev), "vxlan")) {
> + if (data->tnl_type == OVS_VPORT_TYPE_GENEVE ||
> + data->tnl_type == OVS_VPORT_TYPE_VXLAN) {
>
> if (IP_VER(ip->ip_ihl_ver) == 4) {
> dp_packet_hwol_set_tx_ipv4(packet);
> @@ -284,7 +284,7 @@ dp_packet_tnl_ol_process(const struct netdev *netdev,
>
> /* Attention please, tunnel inner l2 len is consist of udp header
> * len and tunnel header len and inner l2 len. */
> - if (!strcmp(netdev_get_type(netdev), "geneve")) {
> + if (data->tnl_type == OVS_VPORT_TYPE_GENEVE) {
> eth = (struct eth_header *)(data->header);
> ip = (struct ip_header *)(eth + 1);
> udp = (struct udp_header *)(ip + 1);
> @@ -292,13 +292,13 @@ dp_packet_tnl_ol_process(const struct netdev *netdev,
> opt_len = gnh->opt_len * 4;
> dp_packet_hwol_set_tunnel_geneve(packet);
> dp_packet_set_l2_len(packet, (char *) dp_packet_l3(packet) -
> - (char *) dp_packet_eth(packet) +
> - GENEVE_BASE_HLEN + opt_len);
> - } else if (!strcmp(netdev_get_type(netdev), "vxlan")) {
> + (char *) dp_packet_eth(packet) +
> + GENEVE_BASE_HLEN + opt_len);
> + } else if (data->tnl_type == OVS_VPORT_TYPE_VXLAN) {
> dp_packet_hwol_set_tunnel_vxlan(packet);
> dp_packet_set_l2_len(packet, (char *) dp_packet_l3(packet) -
> - (char *) dp_packet_eth(packet) +
> - VXLAN_HLEN);
> + (char *) dp_packet_eth(packet) +
> + VXLAN_HLEN);
> }
> }
> }
With the changes above in place the netdev parameter of
dp_packet_tnl_ol_process() is no longer used.
As GitHub actions configure OvS compilation with --enable-Werror
this results in a build error [1].
[1] https://github.com/ovsrobot/ovs/actions/runs/7553819191
In order to move things along I have locally squashed the following into
this patch:
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index d513194dc462..5065a40a7fc0 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -244,8 +244,7 @@ udp_extract_tnl_md(struct dp_packet *packet, struct
flow_tnl *tnl,
/* Calculate inner l2 l3 l4 len as tunnel outer header is not
* encapsulated now. */
static void
-dp_packet_tnl_ol_process(const struct netdev *netdev,
- struct dp_packet *packet,
+dp_packet_tnl_ol_process(struct dp_packet *packet,
const struct ovs_action_push_tnl *data)
{
struct udp_header *udp = NULL;
@@ -304,7 +303,7 @@ dp_packet_tnl_ol_process(const struct netdev *netdev,
}
void
-netdev_tnl_push_udp_header(const struct netdev *netdev,
+netdev_tnl_push_udp_header(const struct netdev *netdev OVS_UNUSED,
struct dp_packet *packet,
const struct ovs_action_push_tnl *data)
{
@@ -313,7 +312,7 @@ netdev_tnl_push_udp_header(const struct netdev *netdev,
uint16_t l3_ofs = packet->l3_ofs;
uint16_t l4_ofs = packet->l4_ofs;
- dp_packet_tnl_ol_process(netdev, packet, data);
+ dp_packet_tnl_ol_process(packet, data);
udp = netdev_tnl_push_ip_header(packet, data->header, data->header_len,
&ip_tot_size, 0);
And pushed it to my own tree to allow GitHub actions to run [2].
As of writhing those tests are still running, but so far there are no
errors.
[2] https://github.com/horms/ovs/actions/runs/7559436191
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
