| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210528155800.0514d249@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date: Fri, 28 May 2021 15:58:00 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Sharath Chandra Vurukala <sharathv@...eaurora.org>
Cc: davem@...emloft.net, elder@...nel.org, cpratapa@...eaurora.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v7 2/3] net: ethernet: rmnet: Support for
ingress MAPv5 checksum offload
On Thu, 27 May 2021 14:18:41 +0530 Sharath Chandra Vurukala wrote:
> Adding support for processing of MAPv5 downlink packets.
> It involves parsing the Mapv5 packet and checking the csum header
> to know whether the hardware has validated the checksum and is
> valid or not.
>
> Based on the checksum valid bit the corresponding stats are
> incremented and skb->ip_summed is marked either CHECKSUM_UNNECESSARY
> or left as CHEKSUM_NONE to let network stack revalidate the checksum
> and update the respective snmp stats.
>
> Current MAPV1 header has been modified, the reserved field in the
> Mapv1 header is now used for next header indication.
>
> Signed-off-by: Sharath Chandra Vurukala <sharathv@...eaurora.org>
> @@ -300,8 +301,11 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
> struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
> struct rmnet_port *port)
> {
> + struct rmnet_map_v5_csum_header *next_hdr = NULL;
> + void *data = skb->data;
> struct rmnet_map_header *maph;
Please maintain reverse xmas tree ordering
> struct sk_buff *skbn;
> + u8 nexthdr_type;
> u32 packet_len;
>
> if (skb->len == 0)
> @@ -310,8 +314,18 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
> maph = (struct rmnet_map_header *)skb->data;
> packet_len = ntohs(maph->pkt_len) + sizeof(*maph);
>
> - if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4)
> + if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) {
> packet_len += sizeof(struct rmnet_map_dl_csum_trailer);
> + } else if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV5) {
> + if (!(maph->flags & MAP_CMD_FLAG)) {
> + packet_len += sizeof(*next_hdr);
> + if (maph->flags & MAP_NEXT_HEADER_FLAG)
> + next_hdr = (data + sizeof(*maph));
brackets unnecessary
> + else
> + /* Mapv5 data pkt without csum hdr is invalid */
> + return NULL;
> + }
> + }
>
> if (((int)skb->len - (int)packet_len) < 0)
> return NULL;
> @@ -320,6 +334,13 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb,
> if (!maph->pkt_len)
> return NULL;
>
> + if (next_hdr) {
> + nexthdr_type = u8_get_bits(next_hdr->header_info,
> + MAPV5_HDRINFO_HDR_TYPE_FMASK);
> + if (nexthdr_type != RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD)
> + return NULL;
> + }
> +
> skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
> if (!skbn)
> return NULL;
> @@ -414,3 +435,37 @@ void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
>
> priv->stats.csum_sw++;
> }
> +
> +/* Process a MAPv5 packet header */
> +int rmnet_map_process_next_hdr_packet(struct sk_buff *skb,
> + u16 len)
> +{
> + struct rmnet_priv *priv = netdev_priv(skb->dev);
> + struct rmnet_map_v5_csum_header *next_hdr;
> + u8 nexthdr_type;
> + int rc = 0;
rc is not meaningfully used
> + next_hdr = (struct rmnet_map_v5_csum_header *)(skb->data +
> + sizeof(struct rmnet_map_header));
> +
> + nexthdr_type = u8_get_bits(next_hdr->header_info,
> + MAPV5_HDRINFO_HDR_TYPE_FMASK);
> +
> + if (nexthdr_type == RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD) {
> + if (unlikely(!(skb->dev->features & NETIF_F_RXCSUM))) {
> + priv->stats.csum_sw++;
> + } else if (next_hdr->csum_info & MAPV5_CSUMINFO_VALID_FLAG) {
> + priv->stats.csum_ok++;
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> + } else {
> + priv->stats.csum_valid_unset++;
> + }
> +
> + /* Pull csum v5 header */
> + skb_pull(skb, sizeof(*next_hdr));
> + } else {
> + return -EINVAL;
flip condition, return early
> + }
> +
> + return rc;
> +}
> diff --git a/include/linux/if_rmnet.h b/include/linux/if_rmnet.h
> index 4efb537..8502ccc 100644
> --- a/include/linux/if_rmnet.h
> +++ b/include/linux/if_rmnet.h
> @@ -1,5 +1,5 @@
> /* SPDX-License-Identifier: GPL-2.0-only
> - * Copyright (c) 2013-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2013-2019, 2021 The Linux Foundation. All rights reserved.
> */
>
> #ifndef _LINUX_IF_RMNET_H_
> @@ -14,8 +14,10 @@ struct rmnet_map_header {
> /* rmnet_map_header flags field:
> * PAD_LEN: number of pad bytes following packet data
> * CMD: 1 = packet contains a MAP command; 0 = packet contains data
> + * NEXT_HEADER 1 = packet contains V5 CSUM header 0 = no V5 CSUM header
Colon missing?
> */
> #define MAP_PAD_LEN_MASK GENMASK(5, 0)
> +#define MAP_NEXT_HEADER_FLAG BIT(6)
> #define MAP_CMD_FLAG BIT(7)
>
> struct rmnet_map_dl_csum_trailer {
> @@ -45,4 +47,26 @@ struct rmnet_map_ul_csum_header {
> #define MAP_CSUM_UL_UDP_FLAG BIT(14)
> #define MAP_CSUM_UL_ENABLED_FLAG BIT(15)
>
> +/* MAP CSUM headers */
> +struct rmnet_map_v5_csum_header {
> + u8 header_info;
> + u8 csum_info;
> + __be16 reserved;
> +} __aligned(1);
__aligned() seems rather pointless here but ok.
> +/* v5 header_info field
> + * NEXT_HEADER: Represents whether there is any other header
double space
> + * HEADER TYPE: represents the type of this header
On previous line you used _ for a space, and started from capital
letter. Please be consistent.
> + *
> + * csum_info field
> + * CSUM_VALID_OR_REQ:
> + * 1 = for UL, checksum computation is requested.
> + * 1 = for DL, validated the checksum and has found it valid
> + */
> +
> +#define MAPV5_HDRINFO_NXT_HDR_FLAG BIT(0)
> +#define MAPV5_HDRINFO_HDR_TYPE_FMASK GENMASK(7, 1)
> +#define MAPV5_CSUMINFO_VALID_FLAG BIT(7)
> +
> +#define RMNET_MAP_HEADER_TYPE_CSUM_OFFLOAD 2
> #endif /* !(_LINUX_IF_RMNET_H_) */
Powered by blists - more mailing lists