re iproute2 - don't return error on success fix

re iproute2 - don't return error on success fix

[Or Gerlitz]

Something is still broken also after commit b45e300 "libnetlink: don't
return error on success" - when error is returned, the error code is
success..

$ tc filter add dev enp33s0f0 protocol ip parent ffff: flower skip_sw
ip_flags nofirstfrag action drop && echo "success" || echo "failed"

RTNETLINK answers: Operation not supported
success

Re: re iproute2 - don't return error on success fix

[Stephen Hemminger]

On Thu, 27 Sep 2018 15:22:41 +0300
Or Gerlitz <gerlitz.or@gmail.com> wrote:

> Something is still broken also after commit b45e300 "libnetlink: don't
> return error on success" - when error is returned, the error code is
> success..
> 
> $ tc filter add dev enp33s0f0 protocol ip parent ffff: flower skip_sw
> ip_flags nofirstfrag action drop && echo "success" || echo "failed"
> 
Yes this looks broken. Let me check.

Re: re iproute2 - don't return error on success fix

[Phil Sutter]

Hi,

On Thu, Sep 27, 2018 at 03:22:41PM +0300, Or Gerlitz wrote:
> Something is still broken also after commit b45e300 "libnetlink: don't
> return error on success" - when error is returned, the error code is
> success..
> 
> $ tc filter add dev enp33s0f0 protocol ip parent ffff: flower skip_sw
> ip_flags nofirstfrag action drop && echo "success" || echo "failed"
> 
> RTNETLINK answers: Operation not supported
> success

Hmm, I can't reproduce this. My HEAD is at the commit you mentioned:

| % sudo ./tc/tc filter add dev d0 protocol ip parent ffff: flower skip_sw ip_flags nofirstfrag action drop
| RTNETLINK answers: Operation not supported
| We have an error talking to the kernel, -1
| % echo $?
| 2

Are you sure you tested the right binary?

Cheers, Phil

Re: re iproute2 - don't return error on success fix

[Or Gerlitz]

On Thu, Sep 27, 2018 at 3:53 PM Phil Sutter <phil@nwl.cc> wrote:

> Hmm, I can't reproduce this. My HEAD is at the commit you mentioned:
>
> | % sudo ./tc/tc filter add dev d0 protocol ip parent ffff: flower skip_sw ip_flags nofirstfrag action drop
> | RTNETLINK answers: Operation not supported
> | We have an error talking to the kernel, -1
> | % echo $?
> | 2
>
> Are you sure you tested the right binary?

I will double check, but we're on weekend + holiday, so I'll get doing
that next week

Re: re iproute2 - don't return error on success fix

[Or Gerlitz]

On Thu, Sep 27, 2018 at 3:53 PM Phil Sutter <phil@nwl.cc> wrote:
> On Thu, Sep 27, 2018 at 03:22:41PM +0300, Or Gerlitz wrote:
> > Something is still broken also after commit b45e300 "libnetlink: don't
> > return error on success" - when error is returned, the error code is
> > success..
> >
> > $ tc filter add dev enp33s0f0 protocol ip parent ffff: flower skip_sw
> > ip_flags nofirstfrag action drop && echo "success" || echo "failed"
> >
> > RTNETLINK answers: Operation not supported
> > success
>
> Hmm, I can't reproduce this. My HEAD is at the commit you mentioned:
>
> | % sudo ./tc/tc filter add dev d0 protocol ip parent ffff: flower skip_sw ip_flags nofirstfrag action drop
> | RTNETLINK answers: Operation not supported
> | We have an error talking to the kernel, -1
> | % echo $?
> | 2
>
> Are you sure you tested the right binary?

double checked now, seems broken on my end:

$ git log --oneline -3
b45e300 libnetlink: don't return error on success
5dc2204 testsuite: add libmnl
8804a8c Makefile: Add check target

$./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
skip_sw ip_flags frag action drop && echo "success" || echo "failed"
success

$ ./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
skip_sw ip_flags firstfrag action drop && echo "success" || echo
"failed"
RTNETLINK answers: Operation not supported
success

$ ./tc/tc filter show dev enp33s0f0 ingress
filter protocol ip pref 49152 flower chain 0
filter protocol ip pref 49152 flower chain 0 handle 0x1
  eth_type ipv4
  ip_flags frag
  skip_sw
  in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1

Re: re iproute2 - don't return error on success fix

[Phil Sutter]

Hi Or,

On Tue, Oct 02, 2018 at 03:21:18PM +0300, Or Gerlitz wrote:
> On Thu, Sep 27, 2018 at 3:53 PM Phil Sutter <phil@nwl.cc> wrote:
[...]
> > Hmm, I can't reproduce this. My HEAD is at the commit you mentioned:
> >
> > | % sudo ./tc/tc filter add dev d0 protocol ip parent ffff: flower skip_sw ip_flags nofirstfrag action drop
> > | RTNETLINK answers: Operation not supported
> > | We have an error talking to the kernel, -1
> > | % echo $?
> > | 2
> >
> > Are you sure you tested the right binary?
> 
> double checked now, seems broken on my end:
> 
> $ git log --oneline -3
> b45e300 libnetlink: don't return error on success
> 5dc2204 testsuite: add libmnl
> 8804a8c Makefile: Add check target
> 
> $./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
> skip_sw ip_flags frag action drop && echo "success" || echo "failed"
> success
> 
> $ ./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
> skip_sw ip_flags firstfrag action drop && echo "success" || echo
> "failed"
> RTNETLINK answers: Operation not supported
> success

Interestingly, your output lacks the "We have an error" message mine
shows. So in your case, the call to rtnl_talk_iov() from
tc_filter_modify() does not return < 0.

In my case, commands always fail with 'skip_sw' since I only test on a
dummy interface. So maybe we hit different error paths? Could you please
check what happens for you in __rtnl_talk_iov()? I guess the "RTNETLINK
answers:" message should come from rtnl_talk_error(). In my case it does
at least, and I haven't found an alternative place where that message
could come from.

Cheers, Phil

Re: re iproute2 - don't return error on success fix

[Or Gerlitz]

On Mon, Oct 8, 2018 at 3:41 PM Phil Sutter <phil@nwl.cc> wrote:

> > $ ./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
> > skip_sw ip_flags firstfrag action drop && echo "success" || echo
> > "failed"
> > RTNETLINK answers: Operation not supported
> > success
>
> Interestingly, your output lacks the "We have an error" message mine
> shows. So in your case, the call to rtnl_talk_iov() from
> tc_filter_modify() does not return < 0.
>
> In my case, commands always fail with 'skip_sw' since I only test on a
> dummy interface. So maybe we hit different error paths? Could you please
> check what happens for you in __rtnl_talk_iov()? I guess the "RTNETLINK
> answers:" message should come from rtnl_talk_error(). In my case it does
> at least, and I haven't found an alternative place where that message
> could come from.

I have applied the patch Vlad sent today and it solved the problem:

# ./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
skip_sw ip_flags firstfrag action drop && echo "success" || echo
"failed"
RTNETLINK answers: Operation not supported
We have an error talking to the kernel, -1
failed