Re: [PATCH net-next 08/10] hv_netvsc: Don't ask for additional head room in the skb

From: Vitaly Kuznetsov
Date: Tue Nov 24 2015 - 03:56:29 EST


"K. Y. Srinivasan" <kys@xxxxxxxxxxxxx> writes:

> The rndis header is 116 bytes big and can be placed in the default
> head room that will be available in the skb.

We have the following in include/linux/netdevice.h:

#if defined(CONFIG_WLAN) || IS_ENABLED(CONFIG_AX25)
# if defined(CONFIG_MAC80211_MESH)
# define LL_MAX_HEADER 128
# else
# define LL_MAX_HEADER 96
# endif
#else
# define LL_MAX_HEADER 32
#endif

In case someone disables MAC80211_MESH in his kernel config we're in
troubles again. I suggest we add additional patch here making sure it is
128 bytes for Hyper-V:

#if defined(CONFIG_HYPERV_NET)
# define LL_MAX_HEADER 128
#elseif defined(CONFIG_WLAN) || IS_ENABLED(CONFIG_AX25)
...

> Since the netvsc packet
> is less than 48 bytes, we can use the skb control buffer
> for the netvsc packet. With these changes we don't need to
> ask for additional head room.
>
> Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> Reviewed-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> ---
> drivers/net/hyperv/hyperv_net.h | 3 +++
> drivers/net/hyperv/netvsc_drv.c | 28 +++++++++-------------------
> 2 files changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 9504ca9..e15dc2c 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -124,6 +124,9 @@ struct ndis_tcp_ip_checksum_info;
> /*
> * Represent netvsc packet which contains 1 RNDIS and 1 ethernet frame
> * within the RNDIS
> + *
> + * The size of this structure is less than 48 bytes and we can now
> + * place this structure in the skb->cb field.
> */
> struct hv_netvsc_packet {
> /* Bookkeeping stuff */
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 947b778..9b6c507 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -432,7 +432,6 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
> u32 net_trans_info;
> u32 hash;
> u32 skb_length;
> - u32 pkt_sz;
> struct hv_page_buffer page_buf[MAX_PAGE_BUFFER_COUNT];
> struct netvsc_stats *tx_stats = this_cpu_ptr(net_device_ctx->tx_stats);
>
> @@ -460,16 +459,19 @@ check_size:
> goto check_size;
> }
>
> - pkt_sz = sizeof(struct hv_netvsc_packet) + RNDIS_AND_PPI_SIZE;
> -
> - ret = skb_cow_head(skb, pkt_sz);
> + /*
> + * Place the rndis header in the skb head room and
> + * the skb->cb will be used for hv_netvsc_packet
> + * structure.
> + */
> + ret = skb_cow_head(skb, RNDIS_AND_PPI_SIZE);
> if (ret) {
> netdev_err(net, "unable to alloc hv_netvsc_packet\n");
> ret = -ENOMEM;
> goto drop;
> }
> - /* Use the headroom for building up the packet */
> - packet = (struct hv_netvsc_packet *)skb->head;
> + /* Use the skb control buffer for building up the packet */
> + packet = (struct hv_netvsc_packet *)skb->cb;
>
> packet->status = 0;
> packet->xmit_more = skb->xmit_more;
> @@ -482,8 +484,7 @@ check_size:
> packet->is_data_pkt = true;
> packet->total_data_buflen = skb->len;
>
> - rndis_msg = (struct rndis_message *)((unsigned long)packet +
> - sizeof(struct hv_netvsc_packet));
> + rndis_msg = (struct rndis_message *)skb->head;
>
> memset(rndis_msg, 0, RNDIS_AND_PPI_SIZE);
>
> @@ -1071,16 +1072,12 @@ static int netvsc_probe(struct hv_device *dev,
> struct netvsc_device_info device_info;
> struct netvsc_device *nvdev;
> int ret;
> - u32 max_needed_headroom;
>
> net = alloc_etherdev_mq(sizeof(struct net_device_context),
> num_online_cpus());
> if (!net)
> return -ENOMEM;
>
> - max_needed_headroom = sizeof(struct hv_netvsc_packet) +
> - RNDIS_AND_PPI_SIZE;
> -
> netif_carrier_off(net);
>
> net_device_ctx = netdev_priv(net);
> @@ -1116,13 +1113,6 @@ static int netvsc_probe(struct hv_device *dev,
> net->ethtool_ops = &ethtool_ops;
> SET_NETDEV_DEV(net, &dev->device);
>
> - /*
> - * Request additional head room in the skb.
> - * We will use this space to build the rndis
> - * heaser and other state we need to maintain.
> - */
> - net->needed_headroom = max_needed_headroom;
> -
> /* Notify the netvsc driver of the new device */
> memset(&device_info, 0, sizeof(device_info));
> device_info.ring_size = ring_size;

--
Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/