[PATCH 00/12] xen: add common function for reading optional value

[PATCH 00/12] xen: add common function for reading optional value

[Juergen Gross]

There are multiple instances of code reading an optional unsigned
parameter from Xenstore via xenbus_scanf(). Instead of repeating the
same code over and over add a service function doing the job and
replace the call of xenbus_scanf() with the call of the new function
where appropriate.

Juergen Gross (12):
  xen: introduce xenbus_read_unsigned()
  xen: make use of xenbus_read_unsigned() in xen-blkback
  xen: make use of xenbus_read_unsigned() in xen-blkfront
  xen: make use of xenbus_read_unsigned() in xen-tpmfront
  xen: make use of xenbus_read_unsigned() in xen-kbdfront
  xen: make use of xenbus_read_unsigned() in xen-netback
  xen: make use of xenbus_read_unsigned() in xen-netfront
  xen: make use of xenbus_read_unsigned() in xen-pcifront
  xen: make use of xenbus_read_unsigned() in xen-scsifront
  xen: make use of xenbus_read_unsigned() in xen-fbfront
  xen: make use of xenbus_read_unsigned() in xen-pciback
  xen: make use of xenbus_read_unsigned() in xenbus

 drivers/block/xen-blkback/xenbus.c        | 36 ++++++--------
 drivers/block/xen-blkfront.c              | 81 ++++++++++---------------------
 drivers/char/tpm/xen-tpmfront.c           |  8 +--
 drivers/input/misc/xen-kbdfront.c         | 13 ++---
 drivers/net/xen-netback/xenbus.c          | 50 ++++++-------------
 drivers/net/xen-netfront.c                | 67 +++++++------------------
 drivers/pci/xen-pcifront.c                |  6 +--
 drivers/scsi/xen-scsifront.c              |  6 +--
 drivers/video/fbdev/xen-fbfront.c         | 13 ++---
 drivers/xen/xen-pciback/xenbus.c          |  8 ++-
 drivers/xen/xenbus/xenbus_probe_backend.c |  8 +--
 drivers/xen/xenbus/xenbus_xs.c            | 22 +++++++--
 include/xen/xenbus.h                      |  4 ++
 13 files changed, 112 insertions(+), 210 deletions(-)

Cc: konrad.wilk@oracle.com
Cc: roger.pau@citrix.com
Cc: peterhuewe@gmx.de
Cc: tpmdd@selhorst.net
Cc: jarkko.sakkinen@linux.intel.com
Cc: jgunthorpe@obsidianresearch.com
Cc: tpmdd-devel@lists.sourceforge.net
Cc: dmitry.torokhov@gmail.com
Cc: linux-input@vger.kernel.org
Cc: wei.liu2@citrix.com
Cc: paul.durrant@citrix.com
Cc: netdev@vger.kernel.org
Cc: bhelgaas@google.com
Cc: linux-pci@vger.kernel.org
Cc: tomi.valkeinen@ti.com
Cc: linux-fbdev@vger.kernel.org
-- 
2.6.6

[PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-netback

[Juergen Gross]

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of some reads from int to unsigned,
but these cases have been wrong before: negative values are not allowed
for the modified cases.

Cc: wei.liu2@citrix.com
Cc: paul.durrant@citrix.com
Cc: netdev@vger.kernel.org

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/net/xen-netback/xenbus.c | 50 +++++++++++-----------------------------
 1 file changed, 14 insertions(+), 36 deletions(-)

diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 8674e18..7356e00 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -785,12 +785,9 @@ static void xen_mcast_ctrl_changed(struct xenbus_watch *watch,
 	struct xenvif *vif = container_of(watch, struct xenvif,
 					  mcast_ctrl_watch);
 	struct xenbus_device *dev = xenvif_to_xenbus_device(vif);
-	int val;
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend,
-			 "request-multicast-control", "%d", &val) < 0)
-		val = 0;
-	vif->multicast_control = !!val;
+	vif->multicast_control = !!xenbus_read_unsigned(dev->otherend,
+					"request-multicast-control", 0);
 }
 
 static int xen_register_mcast_ctrl_watch(struct xenbus_device *dev,
@@ -934,12 +931,9 @@ static void connect(struct backend_info *be)
 	/* Check whether the frontend requested multiple queues
 	 * and read the number requested.
 	 */
-	err = xenbus_scanf(XBT_NIL, dev->otherend,
-			   "multi-queue-num-queues",
-			   "%u", &requested_num_queues);
-	if (err < 0) {
-		requested_num_queues = 1; /* Fall back to single queue */
-	} else if (requested_num_queues > xenvif_max_queues) {
+	requested_num_queues = xenbus_read_unsigned(dev->otherend,
+					"multi-queue-num-queues", 1);
+	if (requested_num_queues > xenvif_max_queues) {
 		/* buggy or malicious guest */
 		xenbus_dev_fatal(dev, err,
 				 "guest requested %u queues, exceeding the maximum of %u.",
@@ -1134,7 +1128,7 @@ static int read_xenbus_vif_flags(struct backend_info *be)
 	struct xenvif *vif = be->vif;
 	struct xenbus_device *dev = be->dev;
 	unsigned int rx_copy;
-	int err, val;
+	int err;
 
 	err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy", "%u",
 			   &rx_copy);
@@ -1150,10 +1144,7 @@ static int read_xenbus_vif_flags(struct backend_info *be)
 	if (!rx_copy)
 		return -EOPNOTSUPP;
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend,
-			 "feature-rx-notify", "%d", &val) < 0)
-		val = 0;
-	if (!val) {
+	if (!xenbus_read_unsigned(dev->otherend, "feature-rx-notify", 0)) {
 		/* - Reduce drain timeout to poll more frequently for
 		 *   Rx requests.
 		 * - Disable Rx stall detection.
@@ -1162,34 +1153,21 @@ static int read_xenbus_vif_flags(struct backend_info *be)
 		be->vif->stall_timeout = 0;
 	}
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
-			 "%d", &val) < 0)
-		val = 0;
-	vif->can_sg = !!val;
+	vif->can_sg = !!xenbus_read_unsigned(dev->otherend, "feature-sg", 0);
 
 	vif->gso_mask = 0;
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
-			 "%d", &val) < 0)
-		val = 0;
-	if (val)
+	if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv4", 0))
 		vif->gso_mask |= GSO_BIT(TCPV4);
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv6",
-			 "%d", &val) < 0)
-		val = 0;
-	if (val)
+	if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv6", 0))
 		vif->gso_mask |= GSO_BIT(TCPV6);
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
-			 "%d", &val) < 0)
-		val = 0;
-	vif->ip_csum = !val;
+	vif->ip_csum = !xenbus_read_unsigned(dev->otherend,
+					     "feature-no-csum-offload", 0);
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum-offload",
-			 "%d", &val) < 0)
-		val = 0;
-	vif->ipv6_csum = !!val;
+	vif->ipv6_csum = !!xenbus_read_unsigned(dev->otherend,
+						"feature-ipv6-csum-offload", 0);
 
 	return 0;
 }
-- 
2.6.6

[PATCH 07/12] xen: make use of xenbus_read_unsigned() in xen-netfront

[Juergen Gross]

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of some reads from int to unsigned,
but these cases have been wrong before: negative values are not allowed
for the modified cases.

Cc: netdev@vger.kernel.org

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/net/xen-netfront.c | 67 +++++++++++++---------------------------------
 1 file changed, 18 insertions(+), 49 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index e17879d..95d664e 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1169,43 +1169,23 @@ static netdev_features_t xennet_fix_features(struct net_device *dev,
 	netdev_features_t features)
 {
 	struct netfront_info *np = netdev_priv(dev);
-	int val;
 
-	if (features & NETIF_F_SG) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
-				 "%d", &val) < 0)
-			val = 0;
+	if (features & NETIF_F_SG &&
+	    !xenbus_read_unsigned(np->xbdev->otherend, "feature-sg", 0))
+		features &= ~NETIF_F_SG;
 
-		if (!val)
-			features &= ~NETIF_F_SG;
-	}
+	if (features & NETIF_F_IPV6_CSUM &&
+	    !xenbus_read_unsigned(np->xbdev->otherend,
+				  "feature-ipv6-csum-offload", 0))
+		features &= ~NETIF_F_IPV6_CSUM;
 
-	if (features & NETIF_F_IPV6_CSUM) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
-				 "feature-ipv6-csum-offload", "%d", &val) < 0)
-			val = 0;
+	if (features & NETIF_F_TSO &&
+	    !xenbus_read_unsigned(np->xbdev->otherend, "feature-gso-tcpv4", 0))
+		features &= ~NETIF_F_TSO;
 
-		if (!val)
-			features &= ~NETIF_F_IPV6_CSUM;
-	}
-
-	if (features & NETIF_F_TSO) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
-				 "feature-gso-tcpv4", "%d", &val) < 0)
-			val = 0;
-
-		if (!val)
-			features &= ~NETIF_F_TSO;
-	}
-
-	if (features & NETIF_F_TSO6) {
-		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
-				 "feature-gso-tcpv6", "%d", &val) < 0)
-			val = 0;
-
-		if (!val)
-			features &= ~NETIF_F_TSO6;
-	}
+	if (features & NETIF_F_TSO6 &&
+	    !xenbus_read_unsigned(np->xbdev->otherend, "feature-gso-tcpv6", 0))
+		features &= ~NETIF_F_TSO6;
 
 	return features;
 }
@@ -1821,18 +1801,13 @@ static int talk_to_netback(struct xenbus_device *dev,
 	info->netdev->irq = 0;
 
 	/* Check if backend supports multiple queues */
-	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
-			   "multi-queue-max-queues", "%u", &max_queues);
-	if (err < 0)
-		max_queues = 1;
+	max_queues = xenbus_read_unsigned(info->xbdev->otherend,
+					  "multi-queue-max-queues", 1);
 	num_queues = min(max_queues, xennet_max_queues);
 
 	/* Check feature-split-event-channels */
-	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
-			   "feature-split-event-channels", "%u",
-			   &feature_split_evtchn);
-	if (err < 0)
-		feature_split_evtchn = 0;
+	feature_split_evtchn = xenbus_read_unsigned(info->xbdev->otherend,
+					"feature-split-event-channels", 0);
 
 	/* Read mac addr. */
 	err = xen_net_read_mac(dev, info->netdev->dev_addr);
@@ -1966,16 +1941,10 @@ static int xennet_connect(struct net_device *dev)
 	struct netfront_info *np = netdev_priv(dev);
 	unsigned int num_queues = 0;
 	int err;
-	unsigned int feature_rx_copy;
 	unsigned int j = 0;
 	struct netfront_queue *queue = NULL;
 
-	err = xenbus_scanf(XBT_NIL, np->xbdev->otherend,
-			   "feature-rx-copy", "%u", &feature_rx_copy);
-	if (err != 1)
-		feature_rx_copy = 0;
-
-	if (!feature_rx_copy) {
+	if (!xenbus_read_unsigned(np->xbdev->otherend, "feature-rx-copy", 0)) {
 		dev_info(&dev->dev,
 			 "backend does not support copying receive path\n");
 		return -ENODEV;
-- 
2.6.6

Re: [PATCH 00/12] xen: add common function for reading optional value

[David Miller]

From: Juergen Gross <jgross@suse.com>
Date: Mon, 31 Oct 2016 17:48:18 +0100

> There are multiple instances of code reading an optional unsigned
> parameter from Xenstore via xenbus_scanf(). Instead of repeating the
> same code over and over add a service function doing the job and
> replace the call of xenbus_scanf() with the call of the new function
> where appropriate.

As this seems to be a series that will go through some tree other
than mine, I assume the networking bits will be taken care of that
way.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 00/12] xen: add common function for reading optional value

[Juergen Gross]

On 31/10/16 18:08, David Miller wrote:
> From: Juergen Gross <jgross@suse.com>
> Date: Mon, 31 Oct 2016 17:48:18 +0100
> 
>> There are multiple instances of code reading an optional unsigned
>> parameter from Xenstore via xenbus_scanf(). Instead of repeating the
>> same code over and over add a service function doing the job and
>> replace the call of xenbus_scanf() with the call of the new function
>> where appropriate.
> 
> As this seems to be a series that will go through some tree other
> than mine, I assume the networking bits will be taken care of that
> way.
> 

If accepted I expect this series to go through the Xen tree.


Juergen

RE: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-netback

[Paul Durrant]

> -----Original Message-----
> From: Juergen Gross [mailto:jgross@suse.com]
> Sent: 31 October 2016 16:48
> To: linux-kernel@vger.kernel.org; xen-devel@lists.xen.org
> Cc: David Vrabel <david.vrabel@citrix.com>; boris.ostrovsky@oracle.com;
> Juergen Gross <jgross@suse.com>; Wei Liu <wei.liu2@citrix.com>; Paul
> Durrant <Paul.Durrant@citrix.com>; netdev@vger.kernel.org
> Subject: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-
> netback
> 
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of some reads from int to unsigned,
> but these cases have been wrong before: negative values are not allowed
> for the modified cases.
> 
> Cc: wei.liu2@citrix.com
> Cc: paul.durrant@citrix.com

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> Cc: netdev@vger.kernel.org
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  drivers/net/xen-netback/xenbus.c | 50 +++++++++++---------------------------
> --
>  1 file changed, 14 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index 8674e18..7356e00 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -785,12 +785,9 @@ static void xen_mcast_ctrl_changed(struct
> xenbus_watch *watch,
>  	struct xenvif *vif = container_of(watch, struct xenvif,
>  					  mcast_ctrl_watch);
>  	struct xenbus_device *dev = xenvif_to_xenbus_device(vif);
> -	int val;
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend,
> -			 "request-multicast-control", "%d", &val) < 0)
> -		val = 0;
> -	vif->multicast_control = !!val;
> +	vif->multicast_control = !!xenbus_read_unsigned(dev->otherend,
> +					"request-multicast-control", 0);
>  }
> 
>  static int xen_register_mcast_ctrl_watch(struct xenbus_device *dev,
> @@ -934,12 +931,9 @@ static void connect(struct backend_info *be)
>  	/* Check whether the frontend requested multiple queues
>  	 * and read the number requested.
>  	 */
> -	err = xenbus_scanf(XBT_NIL, dev->otherend,
> -			   "multi-queue-num-queues",
> -			   "%u", &requested_num_queues);
> -	if (err < 0) {
> -		requested_num_queues = 1; /* Fall back to single queue */
> -	} else if (requested_num_queues > xenvif_max_queues) {
> +	requested_num_queues = xenbus_read_unsigned(dev->otherend,
> +					"multi-queue-num-queues", 1);
> +	if (requested_num_queues > xenvif_max_queues) {
>  		/* buggy or malicious guest */
>  		xenbus_dev_fatal(dev, err,
>  				 "guest requested %u queues, exceeding the
> maximum of %u.",
> @@ -1134,7 +1128,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>  	struct xenvif *vif = be->vif;
>  	struct xenbus_device *dev = be->dev;
>  	unsigned int rx_copy;
> -	int err, val;
> +	int err;
> 
>  	err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy",
> "%u",
>  			   &rx_copy);
> @@ -1150,10 +1144,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>  	if (!rx_copy)
>  		return -EOPNOTSUPP;
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend,
> -			 "feature-rx-notify", "%d", &val) < 0)
> -		val = 0;
> -	if (!val) {
> +	if (!xenbus_read_unsigned(dev->otherend, "feature-rx-notify", 0)) {
>  		/* - Reduce drain timeout to poll more frequently for
>  		 *   Rx requests.
>  		 * - Disable Rx stall detection.
> @@ -1162,34 +1153,21 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
>  		be->vif->stall_timeout = 0;
>  	}
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	vif->can_sg = !!val;
> +	vif->can_sg = !!xenbus_read_unsigned(dev->otherend, "feature-
> sg", 0);
> 
>  	vif->gso_mask = 0;
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	if (val)
> +	if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv4", 0))
>  		vif->gso_mask |= GSO_BIT(TCPV4);
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv6",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	if (val)
> +	if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv6", 0))
>  		vif->gso_mask |= GSO_BIT(TCPV6);
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-
> offload",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	vif->ip_csum = !val;
> +	vif->ip_csum = !xenbus_read_unsigned(dev->otherend,
> +					     "feature-no-csum-offload", 0);
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum-
> offload",
> -			 "%d", &val) < 0)
> -		val = 0;
> -	vif->ipv6_csum = !!val;
> +	vif->ipv6_csum = !!xenbus_read_unsigned(dev->otherend,
> +						"feature-ipv6-csum-offload",
> 0);
> 
>  	return 0;
>  }
> --
> 2.6.6

Re: [PATCH 00/12] xen: add common function for reading optional value

[David Vrabel]

On 31/10/16 16:48, Juergen Gross wrote:
> There are multiple instances of code reading an optional unsigned
> parameter from Xenstore via xenbus_scanf(). Instead of repeating the
> same code over and over add a service function doing the job and
> replace the call of xenbus_scanf() with the call of the new function
> where appropriate.

Acked-by: David Vrabel <david.vrabel@citrix.com>

Please queue for the next release.

David

Re: [PATCH 00/12] xen: add common function for reading optional value

[Jarkko Sakkinen]

On Mon, Nov 07, 2016 at 11:08:09AM +0000, David Vrabel wrote:
> On 31/10/16 16:48, Juergen Gross wrote:
> > There are multiple instances of code reading an optional unsigned
> > parameter from Xenstore via xenbus_scanf(). Instead of repeating the
> > same code over and over add a service function doing the job and
> > replace the call of xenbus_scanf() with the call of the new function
> > where appropriate.
> 
> Acked-by: David Vrabel <david.vrabel-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
> 
> Please queue for the next release.

If you want this change to tpmdd, please resend it to tpmdd mailing
list and CC it to linux-security-module. Thanks.

> David

/Jarkko

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi