Re: [ovs-dev] [PATCH v5 2/2] dpctl: dpif: allow viewing and configuring dp cache sizes

2021-08-23 Thread Eelco Chaudron



On 5 Aug 2021, at 21:47, Paolo Valerio wrote:

> Eelco Chaudron  writes:
>
>> This patch adds a general way of viewing/configuring datapath
>> cache sizes. With an implementation for the netlink interface.
>>
>> The ovs-dpctl/ovs-appctl show commands will display the
>> current cache sizes configured:
>>
>> ovs-dpctl show
>> system@ovs-system:
>>   lookups: hit:25 missed:63 lost:0
>>   flows: 0
>>   masks: hit:282 total:0 hit/pkt:3.20
>>   cache: hit:4 hit rate:4.54%
>>   caches:
>> masks-cache: size: 256
>>   port 0: ovs-system (internal)
>>   port 1: br-int (internal)
>>   port 2: genev_sys_6081 (geneve: packet_type=ptap)
>>   port 3: br-ex (internal)
>>   port 4: eth2
>>   port 5: sw0p1 (internal)
>>   port 6: sw0p3 (internal)
>>
>> A specific cache can be configured as follows:
>>
>> ovs-appctl dpctl/cache-set-size DP CACHE SIZE
>> ovs-dpctl cache-set-size DP CACHE SIZE
>>
>> For example to disable the cache do:
>>
>> $ ovs-dpctl cache-set-size system@ovs-system masks-cache 0
>> Setting cache size successful, new size 0.
>>
>> Signed-off-by: Eelco Chaudron 
>>
>> ---
>> v2: - Changed precision to 2 digits
>> - Handle missing kernel feature at netlink level
>> v3: - Rebase on the latest master branch
>> v4: - Fixed commit message
>> - Fix issue with resetting user_features
>> v5: - Include the actual resetting user_features fix
>
> Hi Eelco,
>
> thanks for including the user_feautes fix.
>
> The patch looks good to me, and I tested it successfully.
> It just needs a rebase.
> Other than that,
>
> Acked-by: Paolo Valerio 

Thanks for the review! Ilya do you want a new version with the rebase?

//Eelco

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v5 2/2] dpctl: dpif: allow viewing and configuring dp cache sizes

2021-08-05 Thread Paolo Valerio
Eelco Chaudron  writes:

> This patch adds a general way of viewing/configuring datapath
> cache sizes. With an implementation for the netlink interface.
>
> The ovs-dpctl/ovs-appctl show commands will display the
> current cache sizes configured:
>
> ovs-dpctl show
> system@ovs-system:
>   lookups: hit:25 missed:63 lost:0
>   flows: 0
>   masks: hit:282 total:0 hit/pkt:3.20
>   cache: hit:4 hit rate:4.54%
>   caches:
> masks-cache: size: 256
>   port 0: ovs-system (internal)
>   port 1: br-int (internal)
>   port 2: genev_sys_6081 (geneve: packet_type=ptap)
>   port 3: br-ex (internal)
>   port 4: eth2
>   port 5: sw0p1 (internal)
>   port 6: sw0p3 (internal)
>
> A specific cache can be configured as follows:
>
> ovs-appctl dpctl/cache-set-size DP CACHE SIZE
> ovs-dpctl cache-set-size DP CACHE SIZE
>
> For example to disable the cache do:
>
> $ ovs-dpctl cache-set-size system@ovs-system masks-cache 0
> Setting cache size successful, new size 0.
>
> Signed-off-by: Eelco Chaudron 
>
> ---
> v2: - Changed precision to 2 digits
> - Handle missing kernel feature at netlink level
> v3: - Rebase on the latest master branch
> v4: - Fixed commit message
> - Fix issue with resetting user_features
> v5: - Include the actual resetting user_features fix

Hi Eelco,

thanks for including the user_feautes fix.

The patch looks good to me, and I tested it successfully.
It just needs a rebase.
Other than that,

Acked-by: Paolo Valerio 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v5 2/2] dpctl: dpif: allow viewing and configuring dp cache sizes

2021-07-13 Thread Flavio Leitner
On Tue, Jun 01, 2021 at 12:39:19PM +0200, Eelco Chaudron wrote:
> This patch adds a general way of viewing/configuring datapath
> cache sizes. With an implementation for the netlink interface.
> 
> The ovs-dpctl/ovs-appctl show commands will display the
> current cache sizes configured:
> 
> ovs-dpctl show
> system@ovs-system:
>   lookups: hit:25 missed:63 lost:0
>   flows: 0
>   masks: hit:282 total:0 hit/pkt:3.20
>   cache: hit:4 hit rate:4.54%
>   caches:
> masks-cache: size: 256
>   port 0: ovs-system (internal)
>   port 1: br-int (internal)
>   port 2: genev_sys_6081 (geneve: packet_type=ptap)
>   port 3: br-ex (internal)
>   port 4: eth2
>   port 5: sw0p1 (internal)
>   port 6: sw0p3 (internal)
> 
> A specific cache can be configured as follows:
> 
> ovs-appctl dpctl/cache-set-size DP CACHE SIZE
> ovs-dpctl cache-set-size DP CACHE SIZE
> 
> For example to disable the cache do:
> 
> $ ovs-dpctl cache-set-size system@ovs-system masks-cache 0
> Setting cache size successful, new size 0.
> 
> Signed-off-by: Eelco Chaudron 
> 
> ---

I reviewed and tested this. It looks good and works for me.

One concern is that UINT32_MAX is used as reserved number to indicate
no kernel support in OVS but the user can try to set the cache size to
that number which would be valid. However, the kernel has a netlink
policy defining a range of values for the cache size and the maximum
is much much lower than that (4k). 

Acked-by: Flavio Leitner 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v5 2/2] dpctl: dpif: allow viewing and configuring dp cache sizes

2021-06-01 Thread Eelco Chaudron
This patch adds a general way of viewing/configuring datapath
cache sizes. With an implementation for the netlink interface.

The ovs-dpctl/ovs-appctl show commands will display the
current cache sizes configured:

ovs-dpctl show
system@ovs-system:
  lookups: hit:25 missed:63 lost:0
  flows: 0
  masks: hit:282 total:0 hit/pkt:3.20
  cache: hit:4 hit rate:4.54%
  caches:
masks-cache: size: 256
  port 0: ovs-system (internal)
  port 1: br-int (internal)
  port 2: genev_sys_6081 (geneve: packet_type=ptap)
  port 3: br-ex (internal)
  port 4: eth2
  port 5: sw0p1 (internal)
  port 6: sw0p3 (internal)

A specific cache can be configured as follows:

ovs-appctl dpctl/cache-set-size DP CACHE SIZE
ovs-dpctl cache-set-size DP CACHE SIZE

For example to disable the cache do:

$ ovs-dpctl cache-set-size system@ovs-system masks-cache 0
Setting cache size successful, new size 0.

Signed-off-by: Eelco Chaudron 

---
v2: - Changed precision to 2 digits
- Handle missing kernel feature at netlink level
v3: - Rebase on the latest master branch
v4: - Fixed commit message
- Fix issue with resetting user_features
v5: - Include the actual resetting user_features fix

 datapath/linux/compat/include/linux/openvswitch.h |1 
 lib/dpctl.c   |  120 +
 lib/dpctl.man |   14 ++
 lib/dpif-netdev.c |4 +
 lib/dpif-netlink.c|  117 
 lib/dpif-provider.h   |   20 
 lib/dpif.c|   32 ++
 lib/dpif.h|7 +
 tests/system-traffic.at   |   36 ++
 utilities/ovs-dpctl.c |4 +
 10 files changed, 355 insertions(+)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 9caef6fe7..35f32d20c 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -105,6 +105,7 @@ enum ovs_datapath_attr {
OVS_DP_ATTR_MEGAFLOW_STATS, /* struct ovs_dp_megaflow_stats */
OVS_DP_ATTR_USER_FEATURES,  /* OVS_DP_F_*  */
OVS_DP_ATTR_PAD,
+   OVS_DP_ATTR_MASKS_CACHE_SIZE,
__OVS_DP_ATTR_MAX
 };
 
diff --git a/lib/dpctl.c b/lib/dpctl.c
index acc677974..6cba8db51 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -591,6 +591,36 @@ compare_port_nos(const void *a_, const void *b_)
 return a < b ? -1 : a > b;
 }
 
+static void
+show_dpif_cache__(struct dpif *dpif, struct dpctl_params *dpctl_p)
+{
+uint32_t nr_caches;
+int error = dpif_cache_get_supported_levels(dpif, _caches);
+
+if (error || nr_caches == 0) {
+return;
+}
+
+dpctl_print(dpctl_p, "  caches:\n");
+for (int i = 0; i < nr_caches; i++) {
+const char *name;
+uint32_t size;
+
+if (dpif_cache_get_name(dpif, i, ) ||
+dpif_cache_get_size(dpif, i, )) {
+continue;
+}
+dpctl_print(dpctl_p, "%s: size: %u\n", name, size);
+}
+}
+
+static void
+show_dpif_cache(struct dpif *dpif, struct dpctl_params *dpctl_p)
+{
+dpctl_print(dpctl_p, "%s:\n", dpif_name(dpif));
+show_dpif_cache__(dpif, dpctl_p);
+}
+
 static void
 show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p)
 {
@@ -623,6 +653,8 @@ show_dpif(struct dpif *dpif, struct dpctl_params *dpctl_p)
 }
 }
 
+show_dpif_cache__(dpif, dpctl_p);
+
 odp_port_t *port_nos = NULL;
 size_t allocated_port_nos = 0, n_port_nos = 0;
 DPIF_PORT_FOR_EACH (_port, , dpif) {
@@ -2409,6 +2441,92 @@ dpctl_ct_ipf_get_status(int argc, const char *argv[],
 return error;
 }
 
+static int
+dpctl_cache_get_size(int argc, const char *argv[],
+ struct dpctl_params *dpctl_p)
+{
+int error;
+
+if (argc > 1) {
+struct dpif *dpif;
+
+error = parsed_dpif_open(argv[1], false, );
+if (!error) {
+show_dpif_cache(dpif, dpctl_p);
+dpif_close(dpif);
+} else {
+dpctl_error(dpctl_p, error, "Opening datapath %s failed", argv[1]);
+}
+} else {
+error = dps_for_each(dpctl_p, show_dpif_cache);
+}
+
+return error;
+}
+
+static int
+dpctl_cache_set_size(int argc, const char *argv[],
+ struct dpctl_params *dpctl_p)
+{
+int i, error = EINVAL;
+uint32_t nr_caches, size;
+struct dpif *dpif;
+
+if (argc != 4) {
+dpctl_error(dpctl_p, error, "Invalid number of arguments");
+return error;
+}
+
+if (!ovs_scan(argv[3], "%"SCNu32, )) {
+dpctl_error(dpctl_p, error, "size is malformed");
+return error;
+}
+
+error = parsed_dpif_open(argv[1], false, );
+if (error) {
+dpctl_error(dpctl_p, error, "Opening datapath %s failed",
+