[ovs-dev] [PATCH] netdev-vport: Do not log empty warnings on success.

Daniele Di Proietto diproiettod at vmware.com
Thu Jan 12 17:55:10 UTC 2017






On 12/01/2017 03:40, "Fischetti, Antonio" <antonio.fischetti at intel.com> wrote:

>Hi Daniele,
>I've checked that without this patch I was getting
>ERROR: 2316 tests were run,
>75 failed unexpectedly.
>2 tests were skipped.
>
>Instead after applying this patch I get
>ERROR: 2316 tests were run,
>42 failed unexpectedly.
>2 tests were skipped.

I'm curious why these fail for you, they happen to pass on travis or locally.
Maybe this is something worth investigating.

>
>
>In particular, after I apply this patch the following tunnel tests are not failing anymore.
>
>=======================================================================
>tunnel
>
>749: tunnel - input                                  ok
>750: tunnel - ECN decapsulation                      ok
>751: tunnel - output                                 ok
>752: tunnel - unencrypted tunnel and not setting skb_mark ok
>753: tunnel - unencrypted tunnel and setting skb_mark to 1 ok
>754: tunnel - unencrypted tunnel and setting skb_mark to 2 ok
>755: tunnel - ToS and TTL inheritance                ok
>756: tunnel - set_tunnel                             ok
>757: tunnel - key                                    ok
>758: tunnel - key match                              ok
>759: tunnel - Geneve                                 ok
>760: tunnel - VXLAN                                  ok
>761: tunnel - LISP                                   ok
>762: tunnel - different VXLAN UDP port               ok
>763: ofproto-dpif - set_field - tun_src/tun_dst/tun_id ok
>764: tunnel - Geneve metadata                        ok
>765: tunnel - Geneve option present                  ok
>766: tunnel - concomitant IPv6 and IPv4 tunnels      ok
>
>tunnel_push_pop
>
>767: tunnel_push_pop - action                        ok
>768: tunnel_push_pop - packet_out                    ok
>
>tunnel_push_pop_ipv6
>
>769: tunnel_push_pop_ipv6 - action                   ok
>
>1093: ofproto-dpif - truncate and output to gre tunnel ok
>
>1097: ofproto-dpif - sFlow packet sampling - tunnel set ok
>1098: ofproto-dpif - sFlow packet sampling - tunnel push ok
>
>1107: ofproto-dpif - Flow IPFIX sanity check - tunnel set ok
>
>1147: ofproto-dpif megaflow - tunnels                 ok
>
>1154: ofproto-dpif - ofproto-dpif-monitor 1           ok
>1155: ofproto-dpif - ofproto-dpif-monitor 2           ok
>
>=======================================================================
>
>One further comment, this patch fails with ./utilities/checkpatch.py.
>E: No signatures found.
>Warnings: 0, Errors: 1

Mmmh, not sure why it's reporting this error.  There is a signoff line.
I tried to run it and it didn't report anything.

>
>Besides this, it looks ok to me.
>
>Acked-by: Antonio Fischetti <antonio.fischetti at intel.com>

Thanks for the review

>
>
>> -----Original Message-----
>> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
>> bounces at openvswitch.org] On Behalf Of Daniele Di Proietto
>> Sent: Thursday, January 12, 2017 8:24 AM
>> To: dev at openvswitch.org
>> Cc: Daniele Di Proietto <diproiettod at vmware.com>
>> Subject: [ovs-dev] [PATCH] netdev-vport: Do not log empty warnings on
>> success.
>> 
>> set_tunnel_config() always logs a warning, even on success. This
>> shouldn't happen.
>> 
>> Without this, some unit tests fail.
>> 
>> Fixes: 9fff138ec3a6("netdev: Add 'errp' to set_config().")
>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>> ---
>>  lib/netdev-vport.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>> index ad5ffcc81..2db51df72 100644
>> --- a/lib/netdev-vport.c
>> +++ b/lib/netdev-vport.c
>> @@ -561,10 +561,12 @@ set_tunnel_config(struct netdev *dev_, const struct
>> smap *args, char **errp)
>>      err = 0;
>> 
>>  out:
>> -    ds_chomp(&errors, '\n');
>> -    VLOG_WARN("%s", ds_cstr(&errors));
>> -    if (err) {
>> -        *errp = ds_steal_cstr(&errors);
>> +    if (*ds_cstr(&errors)) {
>> +        ds_chomp(&errors, '\n');
>> +        VLOG_WARN("%s", ds_cstr(&errors));
>> +        if (err) {
>> +            *errp = ds_steal_cstr(&errors);
>> +        }
>>      }
>> 
>>      ds_destroy(&errors);
>> --
>> 2.11.0
>> 
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list