2:39 a.m.
Reuse the socket across ioctl calls.
On my machine with one hardware network interface and three "software" ones,
this speeds the udev node_device driver startup 11x (from ~220 ms to ~20 ms)
Ján Tomko (5):
Move struct elem out of virNetDevGetFeatures
Split out virNetDevGetEthtoolFeatures
Split out virNetDevGetEthtoolGFeatures
Return bool in virNetDevFeatureAvailable
Reuse the socket in virNetDevGetFeatures
src/util/virnetdev.c | 212 +++++++++++++++++++++++++++++----------------------
1 file changed, 120 insertions(+), 92 deletions(-)
--
2.7.3
2:39 a.m.
New subject: [libvirt] [PATCH 1/5] Move struct elem out of virNetDevGetFeatures
Rename struct elem to ethtool_to_virnetdev_feature and move it
out of the function to allow reusing it.
---
src/util/virnetdev.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 7db4497..354e6f7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3239,6 +3239,11 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
return ret;
}
+struct ethtool_to_virnetdev_feature {
+ const int cmd;
+ const virNetDevFeature feat;
+};
+
/**
* virNetDevFeatureAvailable
@@ -3303,12 +3308,8 @@ virNetDevGetFeatures(const char *ifname,
# if HAVE_DECL_ETHTOOL_GFEATURES
struct ethtool_gfeatures *g_cmd;
# endif
- struct elem{
- const int cmd;
- const virNetDevFeature feat;
- };
/* legacy ethtool getters */
- struct elem cmds[] = {
+ struct ethtool_to_virnetdev_feature cmds[] = {
{ETHTOOL_GRXCSUM, VIR_NET_DEV_FEAT_GRXCSUM},
{ETHTOOL_GTXCSUM, VIR_NET_DEV_FEAT_GTXCSUM},
{ETHTOOL_GSG, VIR_NET_DEV_FEAT_GSG},
@@ -3339,7 +3340,7 @@ virNetDevGetFeatures(const char *ifname,
# if HAVE_DECL_ETHTOOL_GFLAGS
size_t j = -1;
/* ethtool masks */
- struct elem flags[] = {
+ struct ethtool_to_virnetdev_feature flags[] = {
# if HAVE_DECL_ETH_FLAG_LRO
{ETH_FLAG_LRO, VIR_NET_DEV_FEAT_LRO},
# endif
--
2.7.3
9:25 a.m.
New subject: [libvirt] [PATCH 1/5] Move struct elem out of virNetDevGetFeatures
On Mon, Jun 06, 2016 at 09:39:24 +0200, Ján Tomko wrote:
Rename struct elem to ethtool_to_virnetdev_feature and move it
out of the function to allow reusing it.
---
src/util/virnetdev.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 7db4497..354e6f7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3239,6 +3239,11 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
return ret;
}
+struct ethtool_to_virnetdev_feature {
This name is very unorthodox.
ACK to this patch if you choose a more compliant name.
9:37 a.m.
New subject: [libvirt] [PATCH 1/5] Move struct elem out of virNetDevGetFeatures
On Mon, Jun 06, 2016 at 04:25:02PM +0200, Peter Krempa wrote:
On Mon, Jun 06, 2016 at 09:39:24 +0200, Ján Tomko wrote:
> Rename struct elem to ethtool_to_virnetdev_feature and move it
> out of the function to allow reusing it.
> ---
> src/util/virnetdev.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 7db4497..354e6f7 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -3239,6 +3239,11 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
> return ret;
> }
>
> +struct ethtool_to_virnetdev_feature {
This name is very unorthodox.
ACK to this patch if you choose a more compliant name.
How about virNetDevEthtoolFeatureCmd?
Jan
9:49 a.m.
New subject: [libvirt] [PATCH 1/5] Move struct elem out of virNetDevGetFeatures
On Mon, Jun 06, 2016 at 16:37:15 +0200, Ján Tomko wrote:
On Mon, Jun 06, 2016 at 04:25:02PM +0200, Peter Krempa wrote:
> On Mon, Jun 06, 2016 at 09:39:24 +0200, Ján Tomko wrote:
> > Rename struct elem to ethtool_to_virnetdev_feature and move it
> > out of the function to allow reusing it.
> > ---
> > src/util/virnetdev.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> > index 7db4497..354e6f7 100644
> > --- a/src/util/virnetdev.c
> > +++ b/src/util/virnetdev.c
> > @@ -3239,6 +3239,11 @@ virNetDevSendEthtoolIoctl(const char *ifname, void
*cmd)
> > return ret;
> > }
> >
> > +struct ethtool_to_virnetdev_feature {
>
> This name is very unorthodox.
>
> ACK to this patch if you choose a more compliant name.
How about virNetDevEthtoolFeatureCmd?
That sounds good
2:39 a.m.
New subject: [libvirt] [PATCH 2/5] Split out virNetDevGetEthtoolFeatures
Split out the features that we probe via various ethtool commands
and ETHTOOL_GFLAGS.
---
src/util/virnetdev.c | 105 ++++++++++++++++++++++++++++-----------------------
1 file changed, 57 insertions(+), 48 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 354e6f7..210fcda 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3266,6 +3266,62 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value
*cmd)
}
+static void
+virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
+ const char *ifname)
+{
+ size_t i;
+ struct ethtool_value cmd = { 0 };
+
+ /* legacy ethtool getters */
+ struct ethtool_to_virnetdev_feature ethtool_cmds[] = {
+ {ETHTOOL_GRXCSUM, VIR_NET_DEV_FEAT_GRXCSUM},
+ {ETHTOOL_GTXCSUM, VIR_NET_DEV_FEAT_GTXCSUM},
+ {ETHTOOL_GSG, VIR_NET_DEV_FEAT_GSG},
+ {ETHTOOL_GTSO, VIR_NET_DEV_FEAT_GTSO},
+# if HAVE_DECL_ETHTOOL_GGSO
+ {ETHTOOL_GGSO, VIR_NET_DEV_FEAT_GGSO},
+# endif
+# if HAVE_DECL_ETHTOOL_GGRO
+ {ETHTOOL_GGRO, VIR_NET_DEV_FEAT_GGRO},
+# endif
+ };
+
+# if HAVE_DECL_ETHTOOL_GFLAGS
+ /* ethtool masks */
+ struct ethtool_to_virnetdev_feature flags[] = {
+# if HAVE_DECL_ETH_FLAG_LRO
+ {ETH_FLAG_LRO, VIR_NET_DEV_FEAT_LRO},
+# endif
+# if HAVE_DECL_ETH_FLAG_TXVLAN
+ {ETH_FLAG_RXVLAN, VIR_NET_DEV_FEAT_RXVLAN},
+ {ETH_FLAG_TXVLAN, VIR_NET_DEV_FEAT_TXVLAN},
+# endif
+# if HAVE_DECL_ETH_FLAG_NTUBLE
+ {ETH_FLAG_NTUPLE, VIR_NET_DEV_FEAT_NTUPLE},
+# endif
+# if HAVE_DECL_ETH_FLAG_RXHASH
+ {ETH_FLAG_RXHASH, VIR_NET_DEV_FEAT_RXHASH},
+# endif
+ };
+
+ for (i = 0; i < ARRAY_CARDINALITY(ethtool_cmds); i++) {
+ cmd.cmd = ethtool_cmds[i].cmd;
+ if (virNetDevFeatureAvailable(ifname, &cmd) == 1)
+ ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat));
+ }
+
+ cmd.cmd = ETHTOOL_GFLAGS;
+ if (virNetDevFeatureAvailable(ifname, &cmd) == 1) {
+ for (i = 0; i < ARRAY_CARDINALITY(flags); i++) {
+ if (cmd.data & flags[i].cmd)
+ ignore_value(virBitmapSetBit(bitmap, flags[i].feat));
+ }
+ }
+# endif
+}
+
+
# if HAVE_DECL_ETHTOOL_GFEATURES
/**
* virNetDevGFeatureAvailable
@@ -3303,24 +3359,9 @@ int
virNetDevGetFeatures(const char *ifname,
virBitmapPtr *out)
{
- size_t i = -1;
- struct ethtool_value cmd = { 0 };
# if HAVE_DECL_ETHTOOL_GFEATURES
struct ethtool_gfeatures *g_cmd;
# endif
- /* legacy ethtool getters */
- struct ethtool_to_virnetdev_feature cmds[] = {
- {ETHTOOL_GRXCSUM, VIR_NET_DEV_FEAT_GRXCSUM},
- {ETHTOOL_GTXCSUM, VIR_NET_DEV_FEAT_GTXCSUM},
- {ETHTOOL_GSG, VIR_NET_DEV_FEAT_GSG},
- {ETHTOOL_GTSO, VIR_NET_DEV_FEAT_GTSO},
-# if HAVE_DECL_ETHTOOL_GGSO
- {ETHTOOL_GGSO, VIR_NET_DEV_FEAT_GGSO},
-# endif
-# if HAVE_DECL_ETHTOOL_GGRO
- {ETHTOOL_GGRO, VIR_NET_DEV_FEAT_GGRO},
-# endif
- };
if (!(*out = virBitmapNew(VIR_NET_DEV_FEAT_LAST)))
return -1;
@@ -3331,39 +3372,7 @@ virNetDevGetFeatures(const char *ifname,
return 0;
}
- for (i = 0; i < ARRAY_CARDINALITY(cmds); i++) {
- cmd.cmd = cmds[i].cmd;
- if (virNetDevFeatureAvailable(ifname, &cmd) == 1)
- ignore_value(virBitmapSetBit(*out, cmds[i].feat));
- }
-
-# if HAVE_DECL_ETHTOOL_GFLAGS
- size_t j = -1;
- /* ethtool masks */
- struct ethtool_to_virnetdev_feature flags[] = {
-# if HAVE_DECL_ETH_FLAG_LRO
- {ETH_FLAG_LRO, VIR_NET_DEV_FEAT_LRO},
-# endif
-# if HAVE_DECL_ETH_FLAG_TXVLAN
- {ETH_FLAG_RXVLAN, VIR_NET_DEV_FEAT_RXVLAN},
- {ETH_FLAG_TXVLAN, VIR_NET_DEV_FEAT_TXVLAN},
-# endif
-# if HAVE_DECL_ETH_FLAG_NTUBLE
- {ETH_FLAG_NTUPLE, VIR_NET_DEV_FEAT_NTUPLE},
-# endif
-# if HAVE_DECL_ETH_FLAG_RXHASH
- {ETH_FLAG_RXHASH, VIR_NET_DEV_FEAT_RXHASH},
-# endif
- };
-
- cmd.cmd = ETHTOOL_GFLAGS;
- if (virNetDevFeatureAvailable(ifname, &cmd) == 1) {
- for (j = 0; j < ARRAY_CARDINALITY(flags); j++) {
- if (cmd.data & flags[j].cmd)
- ignore_value(virBitmapSetBit(*out, flags[j].feat));
- }
- }
-# endif
+ virNetDevGetEthtoolFeatures(*out, ifname);
# if HAVE_DECL_ETHTOOL_GFEATURES
if (VIR_ALLOC_VAR(g_cmd,
--
2.7.3
2:25 a.m.
New subject: [libvirt] [PATCH 2/5] Split out virNetDevGetEthtoolFeatures
On Mon, Jun 06, 2016 at 09:39:25 +0200, Ján Tomko wrote:
Split out the features that we probe via various ethtool commands
and ETHTOOL_GFLAGS.
---
src/util/virnetdev.c | 105 ++++++++++++++++++++++++++++-----------------------
1 file changed, 57 insertions(+), 48 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 354e6f7..210fcda 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3266,6 +3266,62 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value
*cmd)
}
+static void
+virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
+ const char *ifname)
+{
+ size_t i;
+ struct ethtool_value cmd = { 0 };
+
+ /* legacy ethtool getters */
+ struct ethtool_to_virnetdev_feature ethtool_cmds[] = {
+ {ETHTOOL_GRXCSUM, VIR_NET_DEV_FEAT_GRXCSUM},
+ {ETHTOOL_GTXCSUM, VIR_NET_DEV_FEAT_GTXCSUM},
+ {ETHTOOL_GSG, VIR_NET_DEV_FEAT_GSG},
+ {ETHTOOL_GTSO, VIR_NET_DEV_FEAT_GTSO},
+# if HAVE_DECL_ETHTOOL_GGSO
+ {ETHTOOL_GGSO, VIR_NET_DEV_FEAT_GGSO},
+# endif
+# if HAVE_DECL_ETHTOOL_GGRO
+ {ETHTOOL_GGRO, VIR_NET_DEV_FEAT_GGRO},
+# endif
+ };
+
+# if HAVE_DECL_ETHTOOL_GFLAGS
+ /* ethtool masks */
+ struct ethtool_to_virnetdev_feature flags[] = {
+# if HAVE_DECL_ETH_FLAG_LRO
+ {ETH_FLAG_LRO, VIR_NET_DEV_FEAT_LRO},
+# endif
+# if HAVE_DECL_ETH_FLAG_TXVLAN
+ {ETH_FLAG_RXVLAN, VIR_NET_DEV_FEAT_RXVLAN},
+ {ETH_FLAG_TXVLAN, VIR_NET_DEV_FEAT_TXVLAN},
+# endif
+# if HAVE_DECL_ETH_FLAG_NTUBLE
+ {ETH_FLAG_NTUPLE, VIR_NET_DEV_FEAT_NTUPLE},
+# endif
+# if HAVE_DECL_ETH_FLAG_RXHASH
+ {ETH_FLAG_RXHASH, VIR_NET_DEV_FEAT_RXHASH},
+# endif
+ };
+
+ for (i = 0; i < ARRAY_CARDINALITY(ethtool_cmds); i++) {
This is guarded by HAVE_DECL_ETHTOOL_GFLAGS so it is not equivalent to
the code below.
+ cmd.cmd = ethtool_cmds[i].cmd;
+ if (virNetDevFeatureAvailable(ifname, &cmd) == 1)
+ ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat));
+ }
+
+ cmd.cmd = ETHTOOL_GFLAGS;
+ if (virNetDevFeatureAvailable(ifname, &cmd) == 1) {
+ for (i = 0; i < ARRAY_CARDINALITY(flags); i++) {
+ if (cmd.data & flags[i].cmd)
+ ignore_value(virBitmapSetBit(bitmap, flags[i].feat));
+ }
+ }
+# endif
+}
ACK if you move the first loop out of HAVE_DECL_ETHTOOL_GFLAGS but you
need to keep the second one in place.
2:39 a.m.
New subject: [libvirt] [PATCH 3/5] Split out virNetDevGetEthtoolGFeatures
Move out the code depending on HAVE_DECL_ETHTOOL_GFEATURES.
---
src/util/virnetdev.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 210fcda..4705891 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3342,6 +3342,32 @@ virNetDevGFeatureAvailable(const char *ifname, struct
ethtool_gfeatures *cmd)
ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
return ret;
}
+
+
+static int
+virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
+ const char *ifname)
+{
+ struct ethtool_gfeatures *g_cmd;
+
+ if (VIR_ALLOC_VAR(g_cmd,
+ struct ethtool_get_features_block, GFEATURES_SIZE) < 0)
+ return -1;
+
+ g_cmd->cmd = ETHTOOL_GFEATURES;
+ g_cmd->size = GFEATURES_SIZE;
+ if (virNetDevGFeatureAvailable(ifname, g_cmd) == 1)
+ ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL));
+ VIR_FREE(g_cmd);
+ return 0;
+}
+# else
+static int
+virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap ATTRIBUTE_UNUSED,
+ const char *ifname ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
# endif
@@ -3359,10 +3385,6 @@ int
virNetDevGetFeatures(const char *ifname,
virBitmapPtr *out)
{
-# if HAVE_DECL_ETHTOOL_GFEATURES
- struct ethtool_gfeatures *g_cmd;
-# endif
-
if (!(*out = virBitmapNew(VIR_NET_DEV_FEAT_LAST)))
return -1;
@@ -3374,16 +3396,8 @@ virNetDevGetFeatures(const char *ifname,
virNetDevGetEthtoolFeatures(*out, ifname);
-# if HAVE_DECL_ETHTOOL_GFEATURES
- if (VIR_ALLOC_VAR(g_cmd,
- struct ethtool_get_features_block, GFEATURES_SIZE) < 0)
+ if (virNetDevGetEthtoolGFeatures(*out, ifname) < 0)
return -1;
- g_cmd->cmd = ETHTOOL_GFEATURES;
- g_cmd->size = GFEATURES_SIZE;
- if (virNetDevGFeatureAvailable(ifname, g_cmd) == 1)
- ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_TXUDPTNL));
- VIR_FREE(g_cmd);
-# endif
if (virNetDevRDMAFeature(ifname, out) < 0)
return -1;
--
2.7.3
2:28 a.m.
New subject: [libvirt] [PATCH 3/5] Split out virNetDevGetEthtoolGFeatures
On Mon, Jun 06, 2016 at 09:39:26 +0200, Ján Tomko wrote:
Move out the code depending on HAVE_DECL_ETHTOOL_GFEATURES.
---
src/util/virnetdev.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
ACK
2:39 a.m.
New subject: [libvirt] [PATCH 4/5] Return bool in virNetDevFeatureAvailable
Simplify the logic
---
src/util/virnetdev.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 4705891..52f02d3 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3254,15 +3254,14 @@ struct ethtool_to_virnetdev_feature {
*
* Returns 0 if not found, 1 on success, and -1 on failure.
*/
-static int
+static bool
virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
{
- int ret = -1;
-
cmd = (void*)cmd;
- if (!virNetDevSendEthtoolIoctl(ifname, cmd))
- ret = cmd->data > 0 ? 1 : 0;
- return ret;
+ if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0 &&
+ cmd->data > 0)
+ return true;
+ return false;
}
@@ -3307,12 +3306,12 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
for (i = 0; i < ARRAY_CARDINALITY(ethtool_cmds); i++) {
cmd.cmd = ethtool_cmds[i].cmd;
- if (virNetDevFeatureAvailable(ifname, &cmd) == 1)
+ if (virNetDevFeatureAvailable(ifname, &cmd))
ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat));
}
cmd.cmd = ETHTOOL_GFLAGS;
- if (virNetDevFeatureAvailable(ifname, &cmd) == 1) {
+ if (virNetDevFeatureAvailable(ifname, &cmd)) {
for (i = 0; i < ARRAY_CARDINALITY(flags); i++) {
if (cmd.data & flags[i].cmd)
ignore_value(virBitmapSetBit(bitmap, flags[i].feat));
@@ -3332,15 +3331,13 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
*
* Returns 0 if not found, 1 on success, and -1 on failure.
*/
-static int
+static bool
virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd)
{
- int ret = -1;
-
cmd = (void*)cmd;
- if (!virNetDevSendEthtoolIoctl(ifname, cmd))
- ret = FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
- return ret;
+ if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0)
+ return !!FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
+ return false;
}
@@ -3356,7 +3353,7 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
g_cmd->cmd = ETHTOOL_GFEATURES;
g_cmd->size = GFEATURES_SIZE;
- if (virNetDevGFeatureAvailable(ifname, g_cmd) == 1)
+ if (virNetDevGFeatureAvailable(ifname, g_cmd))
ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL));
VIR_FREE(g_cmd);
return 0;
--
2.7.3
2:38 a.m.
New subject: [libvirt] [PATCH 4/5] Return bool in virNetDevFeatureAvailable
On Mon, Jun 06, 2016 at 09:39:27 +0200, Ján Tomko wrote:
Simplify the logic
---
src/util/virnetdev.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 4705891..52f02d3 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3254,15 +3254,14 @@ struct ethtool_to_virnetdev_feature {
*
* Returns 0 if not found, 1 on success, and -1 on failure.
This is no longer valid after this patch.
*/
-static int
+static bool
virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
{
- int ret = -1;
-
cmd = (void*)cmd;
- if (!virNetDevSendEthtoolIoctl(ifname, cmd))
- ret = cmd->data > 0 ? 1 : 0;
- return ret;
+ if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0 &&
+ cmd->data > 0)
+ return true;
+ return false;
}
@@ -3307,12 +3306,12 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
for (i = 0; i < ARRAY_CARDINALITY(ethtool_cmds); i++) {
cmd.cmd = ethtool_cmds[i].cmd;
- if (virNetDevFeatureAvailable(ifname, &cmd) == 1)
+ if (virNetDevFeatureAvailable(ifname, &cmd))
ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat));
}
cmd.cmd = ETHTOOL_GFLAGS;
- if (virNetDevFeatureAvailable(ifname, &cmd) == 1) {
+ if (virNetDevFeatureAvailable(ifname, &cmd)) {
for (i = 0; i < ARRAY_CARDINALITY(flags); i++) {
if (cmd.data & flags[i].cmd)
ignore_value(virBitmapSetBit(bitmap, flags[i].feat));
@@ -3332,15 +3331,13 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
*
* Returns 0 if not found, 1 on success, and -1 on failure.
Neither this.
*/
-static int
ACK with docs fixed.
2:39 a.m.
New subject: [libvirt] [PATCH 5/5] Reuse the socket in virNetDevGetFeatures
This speeds up node_device_udev driver startup 11x.
---
src/util/virnetdev.c | 69 +++++++++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 31 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 52f02d3..7863e8a 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3206,20 +3206,11 @@ virNetDevRDMAFeature(const char *ifname,
* Returns 0 on success, -1 on failure.
*/
static int
-virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
+virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr)
{
int ret = -1;
- int fd = -1;
- struct ifreq ifr;
-
- /* Ultimately uses AF_PACKET for socket which requires privileged
- * daemon support.
- */
- if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
- return ret;
- ifr.ifr_data = cmd;
- ret = ioctl(fd, SIOCETHTOOL, &ifr);
+ ret = ioctl(fd, SIOCETHTOOL, ifr);
if (ret != 0) {
switch (errno) {
case EINVAL: /* kernel doesn't support SIOCETHTOOL */
@@ -3230,12 +3221,10 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
break;
default:
virReportSystemError(errno, "%s", _("ethtool ioctl
error"));
- goto cleanup;
+ break;
}
}
- cleanup:
- VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -3255,10 +3244,10 @@ struct ethtool_to_virnetdev_feature {
* Returns 0 if not found, 1 on success, and -1 on failure.
*/
static bool
-virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
+virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd)
{
- cmd = (void*)cmd;
- if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0 &&
+ ifr->ifr_data = (void*)cmd;
+ if (virNetDevSendEthtoolIoctl(fd, ifr) == 0 &&
cmd->data > 0)
return true;
return false;
@@ -3267,7 +3256,8 @@ virNetDevFeatureAvailable(const char *ifname, struct ethtool_value
*cmd)
static void
virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
- const char *ifname)
+ int fd,
+ struct ifreq *ifr)
{
size_t i;
struct ethtool_value cmd = { 0 };
@@ -3306,12 +3296,12 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
for (i = 0; i < ARRAY_CARDINALITY(ethtool_cmds); i++) {
cmd.cmd = ethtool_cmds[i].cmd;
- if (virNetDevFeatureAvailable(ifname, &cmd))
+ if (virNetDevFeatureAvailable(fd, ifr, &cmd))
ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat));
}
cmd.cmd = ETHTOOL_GFLAGS;
- if (virNetDevFeatureAvailable(ifname, &cmd)) {
+ if (virNetDevFeatureAvailable(fd, ifr, &cmd)) {
for (i = 0; i < ARRAY_CARDINALITY(flags); i++) {
if (cmd.data & flags[i].cmd)
ignore_value(virBitmapSetBit(bitmap, flags[i].feat));
@@ -3332,10 +3322,12 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap,
* Returns 0 if not found, 1 on success, and -1 on failure.
*/
static bool
-virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd)
+virNetDevGFeatureAvailable(int fd,
+ struct ifreq *ifr,
+ struct ethtool_gfeatures *cmd)
{
- cmd = (void*)cmd;
- if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0)
+ ifr->ifr_data = (void*)cmd;
+ if (virNetDevSendEthtoolIoctl(fd, ifr) == 0)
return !!FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
return false;
}
@@ -3343,7 +3335,8 @@ virNetDevGFeatureAvailable(const char *ifname, struct
ethtool_gfeatures *cmd)
static int
virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
- const char *ifname)
+ int fd,
+ struct ifreq *ifr)
{
struct ethtool_gfeatures *g_cmd;
@@ -3353,7 +3346,7 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
g_cmd->cmd = ETHTOOL_GFEATURES;
g_cmd->size = GFEATURES_SIZE;
- if (virNetDevGFeatureAvailable(ifname, g_cmd))
+ if (virNetDevGFeatureAvailable(fd, ifr, g_cmd))
ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL));
VIR_FREE(g_cmd);
return 0;
@@ -3361,7 +3354,8 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
# else
static int
virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap ATTRIBUTE_UNUSED,
- const char *ifname ATTRIBUTE_UNUSED)
+ int fd ATTRIBUTE_UNUSED,
+ struct ifreq *ifr ATTRIBUGE_UNUSED)
{
return 0;
}
@@ -3382,6 +3376,10 @@ int
virNetDevGetFeatures(const char *ifname,
virBitmapPtr *out)
{
+ struct ifreq ifr;
+ int ret = -1;
+ int fd = -1;
+
if (!(*out = virBitmapNew(VIR_NET_DEV_FEAT_LAST)))
return -1;
@@ -3391,14 +3389,23 @@ virNetDevGetFeatures(const char *ifname,
return 0;
}
- virNetDevGetEthtoolFeatures(*out, ifname);
+ /* Ultimately uses AF_PACKET for socket which requires privileged
+ * daemon support.
+ */
+ if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
+ goto cleanup;
- if (virNetDevGetEthtoolGFeatures(*out, ifname) < 0)
- return -1;
+ virNetDevGetEthtoolFeatures(*out, fd, &ifr);
+
+ if (virNetDevGetEthtoolGFeatures(*out, fd, &ifr) < 0)
+ goto cleanup;
if (virNetDevRDMAFeature(ifname, out) < 0)
- return -1;
- return 0;
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ return ret;
}
#else
int
--
2.7.3
3:32 a.m.
New subject: [libvirt] [PATCH 5/5] Reuse the socket in virNetDevGetFeatures
On Mon, Jun 06, 2016 at 09:39:28 +0200, Ján Tomko wrote:
This speeds up node_device_udev driver startup 11x.
---
src/util/virnetdev.c | 69 +++++++++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 31 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 52f02d3..7863e8a 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3206,20 +3206,11 @@ virNetDevRDMAFeature(const char *ifname,
* Returns 0 on success, -1 on failure.
*/
static int
-virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
+virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr)
You changed arguments but didn't fix the comment that documents them.
{
int ret = -1;
- int fd = -1;
- struct ifreq ifr;
-
- /* Ultimately uses AF_PACKET for socket which requires privileged
- * daemon support.
- */
- if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
- return ret;
- ifr.ifr_data = cmd;
- ret = ioctl(fd, SIOCETHTOOL, &ifr);
+ ret = ioctl(fd, SIOCETHTOOL, ifr);
if (ret != 0) {
switch (errno) {
case EINVAL: /* kernel doesn't support SIOCETHTOOL */
@@ -3230,12 +3221,10 @@ virNetDevSendEthtoolIoctl(const char *ifname, void *cmd)
break;
default:
virReportSystemError(errno, "%s", _("ethtool ioctl
error"));
- goto cleanup;
+ break;
These error reports don't make much sense now, but I guess we can keep
them for debugging purposes.
}
}
- cleanup:
- VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -3255,10 +3244,10 @@ struct ethtool_to_virnetdev_feature {
* Returns 0 if not found, 1 on success, and -1 on failure.
Again. Argument change is not documented.
*/
static bool
-virNetDevFeatureAvailable(const char *ifname, struct ethtool_value *cmd)
+virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd)
{
- cmd = (void*)cmd;
- if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0 &&
+ ifr->ifr_data = (void*)cmd;
+ if (virNetDevSendEthtoolIoctl(fd, ifr) == 0 &&
cmd->data > 0)
return true;
return false;
@@ -3332,10 +3322,12 @@ virNetDevGetEthtoolFeatures(virBitmapPtr
bitmap,
* Returns 0 if not found, 1 on success, and -1 on failure.
And here too ...
*/
static bool
-virNetDevGFeatureAvailable(const char *ifname, struct ethtool_gfeatures *cmd)
+virNetDevGFeatureAvailable(int fd,
+ struct ifreq *ifr,
+ struct ethtool_gfeatures *cmd)
{
- cmd = (void*)cmd;
- if (virNetDevSendEthtoolIoctl(ifname, cmd) == 0)
+ ifr->ifr_data = (void*)cmd;
+ if (virNetDevSendEthtoolIoctl(fd, ifr) == 0)
return !!FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active);
return false;
}
@@ -3343,7 +3335,8 @@ virNetDevGFeatureAvailable(const char *ifname, struct
ethtool_gfeatures *cmd)
static int
virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
- const char *ifname)
+ int fd,
+ struct ifreq *ifr)
Broken formatting.
{
struct ethtool_gfeatures *g_cmd;
[...]
@@ -3391,14 +3389,23 @@ virNetDevGetFeatures(const char *ifname,
return 0;
}
- virNetDevGetEthtoolFeatures(*out, ifname);
+ /* Ultimately uses AF_PACKET for socket which requires privileged
+ * daemon support.
+ */
+ if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
+ goto cleanup;
- if (virNetDevGetEthtoolGFeatures(*out, ifname) < 0)
- return -1;
+ virNetDevGetEthtoolFeatures(*out, fd, &ifr);
+
+ if (virNetDevGetEthtoolGFeatures(*out, fd, &ifr) < 0)
+ goto cleanup;
if (virNetDevRDMAFeature(ifname, out) < 0)
- return -1;
- return 0;
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
You forgot to close the socket.
+ return ret;
}
#else
int
ACK with the issues fixed.
3:01 p.m.
New subject: [libvirt] [PATCH 5/5] Reuse the socket in virNetDevGetFeatures
On Mon, Jun 06, Ján Tomko wrote:
virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap ATTRIBUTE_UNUSED,
- const char *ifname ATTRIBUTE_UNUSED)
+ int fd ATTRIBUTE_UNUSED,
+ struct ifreq *ifr ATTRIBUGE_UNUSED)
Typo.
1:46 a.m.
New subject: [libvirt] [PATCH 5/5] Reuse the socket in virNetDevGetFeatures
On Wed, Jun 08, 2016 at 10:01:34PM +0200, Olaf Hering wrote:
On Mon, Jun 06, Ján Tomko wrote:
> virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap ATTRIBUTE_UNUSED,
> - const char *ifname ATTRIBUTE_UNUSED)
> + int fd ATTRIBUTE_UNUSED,
> + struct ifreq *ifr ATTRIBUGE_UNUSED)
Typo.
Thanks; fixed.
Jan
3089
days inactive
3092
days old
14 comments
3 participants
participants (3)
-
Ján Tomko -
Olaf Hering -
Peter Krempa