RE: [PATCH net-next 2/4] xen-netback: add control protocol implementation

[Paul Durrant]

> -Original Message-
> From: Wei Liu [mailto:wei.l...@citrix.com]
> Sent: 10 May 2016 14:29
> To: Paul Durrant
> Cc: xen-de...@lists.xenproject.org; netdev@vger.kernel.org; Wei Liu
> Subject: Re: [PATCH net-next 2/4] xen-netback: add control protocol
> implementation
> 
> On Thu, May 05, 2016 at 12:19:28PM +0100, Paul Durrant wrote:
> > My recent patch to include/xen/interface/io/netif.h defines a new shared
> > ring (in addition to the rx and tx rings) for passing control messages
> > from a VM frontend driver to a backend driver.
> >
> > A previous patch added the necessary boilerplate for mapping the control
> > ring from the frontend, should it be created. This patch adds
> > implementations for each of the defined protocol messages.
> >
> > Signed-off-by: Paul Durrant <paul.durr...@citrix.com>
> > Cc: Wei Liu <wei.l...@citrix.com>
> > ---
> >  drivers/net/xen-netback/Makefile|   2 +-
> >  drivers/net/xen-netback/common.h|  43 +
> >  drivers/net/xen-netback/hash.c  | 361
> 
> >  drivers/net/xen-netback/interface.c |  28 +++
> >  drivers/net/xen-netback/netback.c   |  49 -
> >  5 files changed, 480 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/net/xen-netback/hash.c
> >
> 
> Other than the issue mentioned by David, the code looks OK to me.
> 

Cool, thanks. I should have the RCU-based hash code done in the next day or so.

  Paul

> Wei.

Re: [PATCH net-next 2/4] xen-netback: add control protocol implementation

[Wei Liu]

On Thu, May 05, 2016 at 12:19:28PM +0100, Paul Durrant wrote:
> My recent patch to include/xen/interface/io/netif.h defines a new shared
> ring (in addition to the rx and tx rings) for passing control messages
> from a VM frontend driver to a backend driver.
> 
> A previous patch added the necessary boilerplate for mapping the control
> ring from the frontend, should it be created. This patch adds
> implementations for each of the defined protocol messages.
> 
> Signed-off-by: Paul Durrant 
> Cc: Wei Liu 
> ---
>  drivers/net/xen-netback/Makefile|   2 +-
>  drivers/net/xen-netback/common.h|  43 +
>  drivers/net/xen-netback/hash.c  | 361 
> 
>  drivers/net/xen-netback/interface.c |  28 +++
>  drivers/net/xen-netback/netback.c   |  49 -
>  5 files changed, 480 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/net/xen-netback/hash.c
> 

Other than the issue mentioned by David, the code looks OK to me.

Wei.

RE: [PATCH net-next 2/4] xen-netback: add control protocol implementation

[Paul Durrant]

> -Original Message-
> From: David Miller [mailto:da...@davemloft.net]
> Sent: 07 May 2016 20:09
> To: Paul Durrant
> Cc: xen-de...@lists.xenproject.org; netdev@vger.kernel.org; Wei Liu
> Subject: Re: [PATCH net-next 2/4] xen-netback: add control protocol
> implementation
> 
> From: Paul Durrant <paul.durr...@citrix.com>
> Date: Thu, 5 May 2016 12:19:28 +0100
> 
> > +struct xenvif_hash_cache {
> > +   rwlock_t lock;
> 
> You really don't want to lock on every SKB hash computation like
> this, turn this into a spin lock for locking the write side and
> use RCU locking for lookup and usage.
>

Yes, that would be better. Will do.

Cheers,

  Paul
 
> THanks.

Re: [PATCH net-next 2/4] xen-netback: add control protocol implementation

[David Miller]

From: Paul Durrant 
Date: Thu, 5 May 2016 12:19:28 +0100

> +struct xenvif_hash_cache {
> + rwlock_t lock;

You really don't want to lock on every SKB hash computation like
this, turn this into a spin lock for locking the write side and
use RCU locking for lookup and usage.

THanks.

[PATCH net-next 2/4] xen-netback: add control protocol implementation

[Paul Durrant]

My recent patch to include/xen/interface/io/netif.h defines a new shared
ring (in addition to the rx and tx rings) for passing control messages
from a VM frontend driver to a backend driver.

A previous patch added the necessary boilerplate for mapping the control
ring from the frontend, should it be created. This patch adds
implementations for each of the defined protocol messages.

Signed-off-by: Paul Durrant 
Cc: Wei Liu 
---
 drivers/net/xen-netback/Makefile|   2 +-
 drivers/net/xen-netback/common.h|  43 +
 drivers/net/xen-netback/hash.c  | 361 
 drivers/net/xen-netback/interface.c |  28 +++
 drivers/net/xen-netback/netback.c   |  49 -
 5 files changed, 480 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/xen-netback/hash.c

diff --git a/drivers/net/xen-netback/Makefile b/drivers/net/xen-netback/Makefile
index e346e81..11e02be 100644
--- a/drivers/net/xen-netback/Makefile
+++ b/drivers/net/xen-netback/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_XEN_NETDEV_BACKEND) := xen-netback.o
 
-xen-netback-y := netback.o xenbus.o interface.o
+xen-netback-y := netback.o xenbus.o interface.o hash.o
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 093a12a..4959716 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -220,6 +220,32 @@ struct xenvif_mcast_addr {
 
 #define XEN_NETBK_MCAST_MAX 64
 
+#define XEN_NETBK_MAX_HASH_KEY_SIZE 40
+#define XEN_NETBK_MAX_HASH_MAPPING_SIZE 128
+#define XEN_NETBK_HASH_TAG_SIZE 40
+
+struct xenvif_hash_cache_entry {
+   u8 tag[XEN_NETBK_HASH_TAG_SIZE];
+   unsigned int len;
+   u32 val;
+   int seq;
+};
+
+struct xenvif_hash_cache {
+   rwlock_t lock;
+   struct xenvif_hash_cache_entry *entry;
+   atomic_t seq;
+};
+
+struct xenvif_hash {
+   unsigned int alg;
+   u32 flags;
+   u8 key[XEN_NETBK_MAX_HASH_KEY_SIZE];
+   u32 mapping[XEN_NETBK_MAX_HASH_MAPPING_SIZE];
+   unsigned int size;
+   struct xenvif_hash_cache cache;
+};
+
 struct xenvif {
/* Unique identifier for this interface. */
domid_t  domid;
@@ -251,6 +277,8 @@ struct xenvif {
unsigned int num_queues; /* active queues, resource allocated */
unsigned int stalled_queues;
 
+   struct xenvif_hash hash;
+
struct xenbus_watch credit_watch;
struct xenbus_watch mcast_ctrl_watch;
 
@@ -353,6 +381,7 @@ extern bool separate_tx_rx_irq;
 extern unsigned int rx_drain_timeout_msecs;
 extern unsigned int rx_stall_timeout_msecs;
 extern unsigned int xenvif_max_queues;
+extern unsigned int xenvif_hash_cache_size;
 
 #ifdef CONFIG_DEBUG_FS
 extern struct dentry *xen_netback_dbg_root;
@@ -366,4 +395,18 @@ void xenvif_skb_zerocopy_complete(struct xenvif_queue 
*queue);
 bool xenvif_mcast_match(struct xenvif *vif, const u8 *addr);
 void xenvif_mcast_addr_list_free(struct xenvif *vif);
 
+/* Hash */
+int xenvif_init_hash(struct xenvif *vif);
+void xenvif_deinit_hash(struct xenvif *vif);
+
+u32 xenvif_set_hash_alg(struct xenvif *vif, u32 alg);
+u32 xenvif_get_hash_flags(struct xenvif *vif, u32 *flags);
+u32 xenvif_set_hash_flags(struct xenvif *vif, u32 flags);
+u32 xenvif_set_hash_key(struct xenvif *vif, u32 gref, u32 len);
+u32 xenvif_set_hash_mapping_size(struct xenvif *vif, u32 size);
+u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len,
+   u32 off);
+
+void xenvif_set_skb_hash(struct xenvif *vif, struct sk_buff *skb);
+
 #endif /* __XEN_NETBACK__COMMON_H__ */
diff --git a/drivers/net/xen-netback/hash.c b/drivers/net/xen-netback/hash.c
new file mode 100644
index 000..054bfd9
--- /dev/null
+++ b/drivers/net/xen-netback/hash.c
@@ -0,0 +1,361 @@
+/*
+ * Copyright (c) 2016 Citrix Systems Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Softare Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL