ss seems broken when using multiple filters in the same cmdline
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
iproute2 (Ubuntu) |
Fix Released
|
Medium
|
Stefan Bader | ||
Cosmic |
Won't Fix
|
Medium
|
Rafael David Tinoco | ||
Disco |
Fix Released
|
Medium
|
Rafael David Tinoco | ||
Eoan |
Fix Released
|
Medium
|
Stefan Bader |
Bug Description
[Impact]
* ss won't be able to run commands with single filters inside parentheses, like: "( sport == :X )", for example.
* A workaround is to remove "( )" from single filters, since it looks the issue does not affect 2 filters being put together in the same parentheses, like: " ( X and Y ) and Y " instead of ( X and Y ) and ( Y )".
* CTDB is unable to use ss filter to obtain nodes public IP addresses in order to fail over services (LP: #722201).
[Test Case]
* Having an Ubuntu Cosmic, Disco or Eoan, try to execute the following command:
$ ss -tn state established "( src [172.16.17.2] || src [172.16.17.3] )" "( sport == :2049 )"
Independent of the IPs or ports being used. Copying and pasting the command should be enough for you to know if you are affected.
Bad Result:
ss: bison bellows (while parsing filter): "syntax error!" Sorry.
Usage: ss [ OPTIONS ]
ss [ OPTIONS ] [ FILTER ]
-h, --help this message
...
Expected Result:
Recv-Q Send-Q Local Address:Port Peer Address:Port
...
* test case was discovered during CTDB scripts execution
[Regression Potential]
* biggest risk would be to affect ss interpreter (worst case scenario).
* the proposed patch is based in upstream fix and was tested against the same issue reported as the reproducer (above) and some other generic ss commands.
* any problem here is unlikely to change iproute2 most important command interpreter, "ip", since the patch is applied against ss code.
[Other Info]
ORIGINAL DESCRIPTION:
Investigating an issue for CTDB (LP: #722201), after suggesting a fix on ss syntax to CTDB upstream project, we discovered that "ss" seems to be broken in Ubuntu since Ubuntu Cosmic:
----
# Debian Sid
inaddy@
Recv-Q Send-Q Local Address:Port Peer Address:Port
# Ubuntu Ubuntu 16.04 LTS (Xenial Xerus)
(c)inaddy@xenial:~$ ss -tn state established "( src [172.16.17.2] || src [172.16.17.3] )" "( sport == :2049 )"
Recv-Q Send-Q Local Address:Port Peer Address:Port
# Ubuntu 18.04 LTS (Bionic Beaver)
(c)inaddy@bionic:~$ ss -tn state established "( src [172.16.17.2] || src [172.16.17.3] )" "( sport == :2049 )"
Recv-Q Send-Q Local Address:Port Peer Address:Port
# Ubuntu 18.10 (Cosmic Cuttlefish)
(c)inaddy@cosmic:~$ ss -tn state established "( src [172.16.17.2] || src [172.16.17.3] )" "( sport == :2049 )"
ss: bison bellows (while parsing filter): "syntax error!" Sorry.
Usage: ss [ OPTIONS ]
ss [ OPTIONS ] [ FILTER ]
-h, --help this message
# Ubuntu 19.04 (Disco Dingo)
(c)inaddy@disco:~$ ss -tn state established "( src [172.16.17.2] || src [172.16.17.3] )" "( sport == :2049 )"
ss: bison bellows (while parsing filter): "syntax error!" Sorry.
Usage: ss [ OPTIONS ]
ss [ OPTIONS ] [ FILTER ]
# Ubuntu 19.10 (Eoan Ermine)
(c)inaddy@eoan:~$ ss -tn state established "( src [172.16.17.2] || src [172.16.17.3] )" "( sport == :2049 )"
ss: bison bellows (while parsing filter): "syntax error!" Sorry.
Usage: ss [ OPTIONS ]
ss [ OPTIONS ] [ FILTER ]
----
I have generated a pkg using upstream iproute2 source code and it does not suffer the issue. I have generated a package using Ubuntu cosmic source package, without debian/patches/*, and verified the issue still persists (not being introduced by any of our packages, and being present if vanilla upstream version used in Cosmic).
Related branches
- Andreas Hasenack: Approve
- Christian Ehrhardt (community): Approve
-
Diff: 849 lines (+690/-44)8 files modifieddebian/changelog (+254/-3)
debian/control (+2/-1)
debian/patches/1000-ubuntu-poc-fan-driver.patch (+74/-0)
debian/patches/1001-ubuntu-poc-fan-driver-v3.patch (+134/-0)
debian/patches/1002-ubuntu-poc-fan-driver-vxlan.patch (+178/-0)
debian/patches/1005-lib-suppress-error-msg-when-filling-the-cache.patch (+40/-0)
debian/patches/series (+8/-1)
dev/null (+0/-39)
- Canonical Server: Pending requested
- Canonical Server Core Reviewers: Pending requested
-
Diff: 37985 lines (+15464/-4254) (has conflicts)301 files modified.clang-format (+130/-0)
Makefile (+23/-7)
README.iproute2+tc (+2/-2)
README.lnstat (+8/-9)
bash-completion/tc (+1/-1)
bridge/br_common.h (+9/-13)
bridge/bridge.c (+9/-9)
bridge/fdb.c (+68/-25)
bridge/link.c (+56/-8)
bridge/mdb.c (+80/-25)
bridge/monitor.c (+5/-6)
bridge/vlan.c (+50/-46)
configure (+34/-63)
debian/.gitlab-ci.yml (+20/-0)
debian/changelog (+107/-11)
debian/control (+7/-1)
debian/gbp.conf (+1/-1)
debian/iproute2.install (+1/-0)
debian/patches/1001-ubuntu-poc-fan-driver-v3.patch (+31/-0)
debian/patches/1002-ubuntu-poc-fan-driver-vxlan.patch (+33/-0)
debian/patches/series (+3/-0)
debian/rules (+18/-8)
debian/tests/control (+2/-2)
debian/tests/testsuite.sh (+2/-12)
dev/null (+0/-73)
devlink/Makefile (+3/-4)
devlink/devlink.c (+2201/-409)
doc/actions/actions-general (+54/-55)
doc/actions/gact-usage (+8/-9)
doc/actions/ifb-README (+37/-37)
doc/actions/mirred-usage (+16/-16)
examples/diffserv/Edge2 (+1/-1)
examples/diffserv/Edge31-ca-u32 (+4/-4)
examples/diffserv/Edge31-cb-chains (+2/-2)
examples/diffserv/Edge32-ca-u32 (+4/-4)
examples/diffserv/Edge32-cb-chains (+2/-2)
examples/diffserv/Edge32-cb-u32 (+2/-2)
examples/diffserv/regression-testing (+1/-1)
genl/ctrl.c (+4/-77)
genl/genl.c (+18/-15)
genl/genl_utils.h (+1/-3)
include/SNAPSHOT.h (+1/-1)
include/bpf_elf.h (+9/-0)
include/bpf_util.h (+2/-1)
include/color.h (+10/-2)
include/json_print.h (+6/-3)
include/json_writer.h (+7/-6)
include/libgenl.h (+3/-3)
include/libnetlink.h (+46/-18)
include/ll_map.h (+2/-3)
include/names.h (+0/-1)
include/uapi/linux/bpf.h (+582/-48)
include/uapi/linux/btf.h (+141/-0)
include/uapi/linux/can.h (+1/-1)
include/uapi/linux/devlink.h (+94/-0)
include/uapi/linux/elf-em.h (+3/-0)
include/uapi/linux/gen_stats.h (+1/-0)
include/uapi/linux/icmpv6.h (+2/-0)
include/uapi/linux/if_addr.h (+1/-0)
include/uapi/linux/if_arp.h (+9/-9)
include/uapi/linux/if_bonding.h (+24/-0)
include/uapi/linux/if_bridge.h (+21/-0)
include/uapi/linux/if_link.h (+40/-0)
include/uapi/linux/if_packet.h (+1/-0)
include/uapi/linux/if_tun.h (+1/-0)
include/uapi/linux/if_tunnel.h (+20/-0)
include/uapi/linux/ila.h (+1/-0)
include/uapi/linux/in.h (+12/-7)
include/uapi/linux/in6.h (+2/-0)
include/uapi/linux/inet_diag.h (+11/-5)
include/uapi/linux/ip.h (+1/-0)
include/uapi/linux/l2tp.h (+8/-7)
include/uapi/linux/magic.h (+2/-0)
include/uapi/linux/neighbour.h (+2/-0)
include/uapi/linux/net_namespace.h (+2/-0)
include/uapi/linux/netconf.h (+1/-0)
include/uapi/linux/netfilter.h (+0/-4)
include/uapi/linux/netfilter/ipset/ip_set.h (+12/-7)
include/uapi/linux/netfilter_ipv4.h (+0/-28)
include/uapi/linux/netfilter_ipv6.h (+0/-29)
include/uapi/linux/netlink.h (+1/-0)
include/uapi/linux/pkt_cls.h (+102/-6)
include/uapi/linux/pkt_sched.h (+231/-4)
include/uapi/linux/rtnetlink.h (+7/-0)
include/uapi/linux/sctp.h (+25/-1)
include/uapi/linux/snmp.h (+324/-0)
include/uapi/linux/tc_act/tc_bpf.h (+0/-2)
include/uapi/linux/tc_act/tc_connmark.h (+0/-2)
include/uapi/linux/tc_act/tc_csum.h (+0/-2)
include/uapi/linux/tc_act/tc_gact.h (+0/-1)
include/uapi/linux/tc_act/tc_ife.h (+0/-1)
include/uapi/linux/tc_act/tc_ipt.h (+0/-3)
include/uapi/linux/tc_act/tc_mirred.h (+0/-1)
include/uapi/linux/tc_act/tc_nat.h (+0/-2)
include/uapi/linux/tc_act/tc_pedit.h (+7/-4)
include/uapi/linux/tc_act/tc_sample.h (+0/-2)
include/uapi/linux/tc_act/tc_skbedit.h (+2/-2)
include/uapi/linux/tc_act/tc_skbmod.h (+0/-2)
include/uapi/linux/tc_act/tc_tunnel_key.h (+28/-2)
include/uapi/linux/tc_act/tc_vlan.h (+0/-2)
include/uapi/linux/tcp.h (+10/-1)
include/uapi/linux/tipc_netlink.h (+14/-0)
include/uapi/linux/xdp_diag.h (+72/-0)
include/uapi/linux/xfrm.h (+4/-1)
include/utils.h (+27/-16)
ip/Makefile (+1/-1)
ip/ip.c (+6/-9)
ip/ip_common.h (+18/-28)
ip/ipaddress.c (+142/-77)
ip/ipaddrlabel.c (+5/-5)
ip/ipfou.c (+1/-2)
ip/ipila.c (+2/-3)
ip/ipl2tp.c (+14/-28)
ip/iplink.c (+13/-14)
ip/iplink_bond.c (+164/-3)
ip/iplink_bond_slave.c (+2/-0)
ip/iplink_bridge.c (+106/-68)
ip/iplink_bridge_slave.c (+39/-3)
ip/iplink_can.c (+13/-8)
ip/iplink_geneve.c (+45/-11)
ip/iplink_ipvlan.c (+18/-10)
ip/iplink_vlan.c (+1/-1)
ip/iplink_vxlan.c (+58/-12)
ip/iplink_xdp.c (+61/-12)
ip/iplink_xstats.c (+4/-3)
ip/ipmacsec.c (+17/-16)
ip/ipmaddr.c (+13/-2)
ip/ipmonitor.c (+52/-39)
ip/ipmroute.c (+27/-10)
ip/ipneigh.c (+81/-21)
ip/ipnetconf.c (+11/-9)
ip/ipnetns.c (+61/-29)
ip/ipntable.c (+11/-12)
ip/ipprefix.c (+1/-1)
ip/iproute.c (+97/-86)
ip/iproute_lwtunnel.c (+154/-54)
ip/iprule.c (+158/-53)
ip/ipseg6.c (+2/-3)
ip/iptoken.c (+2/-2)
ip/iptuntap.c (+3/-4)
ip/ipvrf.c (+1/-1)
ip/ipxfrm.c (+57/-28)
ip/link_gre.c (+6/-6)
ip/link_gre6.c (+5/-5)
ip/link_ip6tnl.c (+4/-4)
ip/link_iptnl.c (+5/-5)
ip/link_vti.c (+3/-3)
ip/link_vti6.c (+3/-3)
ip/link_xfrm.c (+77/-0)
ip/rtmon.c (+7/-6)
ip/tcp_metrics.c (+2/-3)
ip/tunnel.c (+2/-3)
ip/xfrm.h (+5/-15)
ip/xfrm_monitor.c (+21/-25)
ip/xfrm_policy.c (+19/-8)
ip/xfrm_state.c (+51/-13)
lib/bpf.c (+555/-144)
lib/color.c (+45/-2)
lib/json_print.c (+13/-4)
lib/json_writer.c (+15/-17)
lib/libnetlink.c (+329/-74)
lib/ll_addr.c (+13/-11)
lib/ll_map.c (+68/-5)
lib/ll_types.c (+1/-1)
lib/names.c (+0/-28)
lib/utils.c (+143/-52)
man/Makefile (+7/-2)
man/man3/Makefile (+5/-1)
man/man7/Makefile (+5/-1)
man/man8/Makefile (+5/-1)
man/man8/bridge.8 (+75/-12)
man/man8/devlink-dev.8 (+115/-0)
man/man8/devlink-health.8 (+197/-0)
man/man8/devlink-region.8 (+131/-0)
man/man8/devlink-sb.8 (+10/-0)
man/man8/devlink.8 (+15/-1)
man/man8/ifstat.8 (+8/-0)
man/man8/ip-address.8.in (+30/-9)
man/man8/ip-link.8.in (+198/-66)
man/man8/ip-neighbour.8 (+17/-1)
man/man8/ip-netns.8 (+10/-0)
man/man8/ip-route.8.in (+29/-18)
man/man8/ip-rule.8 (+3/-1)
man/man8/ip-tunnel.8 (+1/-1)
man/man8/ip-xfrm.8 (+26/-2)
man/man8/ip.8 (+46/-23)
man/man8/rdma-dev.8 (+15/-1)
man/man8/rdma-link.8 (+1/-0)
man/man8/rdma.8 (+1/-0)
man/man8/rtacct.8 (+9/-5)
man/man8/rtpr.8 (+1/-1)
man/man8/ss.8 (+77/-24)
man/man8/tc-cake.8 (+726/-0)
man/man8/tc-etf.8 (+141/-0)
man/man8/tc-flower.8 (+55/-6)
man/man8/tc-fq.8 (+26/-11)
man/man8/tc-mqprio.8 (+1/-1)
man/man8/tc-netem.8 (+39/-1)
man/man8/tc-pie.8 (+18/-22)
man/man8/tc-skbprio.8 (+70/-0)
man/man8/tc-taprio.8 (+142/-0)
man/man8/tc-tunnel_key.8 (+22/-4)
man/man8/tc.8 (+39/-2)
man/man8/tipc-link.8 (+10/-0)
man/man8/tipc-nametable.8 (+10/-0)
man/man8/tipc.8 (+9/-0)
misc/Makefile (+4/-4)
misc/arpd.c (+1/-1)
misc/ifstat.c (+5/-7)
misc/nstat.c (+7/-3)
misc/ss.c (+311/-64)
misc/ssfilter.y (+25/-19)
netem/README.distribution (+1/-1)
rdma/Makefile (+6/-3)
rdma/dev.c (+67/-19)
rdma/include/uapi/rdma/ib_user_verbs.h (+38/-7)
rdma/include/uapi/rdma/rdma_netlink.h (+62/-16)
rdma/include/uapi/rdma/rdma_user_cm.h (+4/-0)
rdma/link.c (+41/-14)
rdma/rdma.c (+1/-6)
rdma/rdma.h (+16/-11)
rdma/res-cmid.c (+275/-0)
rdma/res-cq.c (+160/-0)
rdma/res-mr.c (+133/-0)
rdma/res-pd.c (+136/-0)
rdma/res-qp.c (+240/-0)
rdma/res.c (+57/-883)
rdma/res.h (+148/-0)
rdma/utils.c (+77/-24)
tc/Makefile (+9/-5)
tc/em_meta.c (+6/-6)
tc/emp_ematch.y (+2/-2)
tc/f_bpf.c (+1/-1)
tc/f_flower.c (+495/-29)
tc/f_matchall.c (+14/-0)
tc/f_u32.c (+1/-5)
tc/m_action.c (+5/-7)
tc/m_bpf.c (+17/-15)
tc/m_connmark.c (+2/-2)
tc/m_csum.c (+2/-1)
tc/m_ematch.c (+4/-27)
tc/m_ematch.h (+15/-20)
tc/m_ife.c (+1/-1)
tc/m_ipt.c (+1/-2)
tc/m_nat.c (+19/-13)
tc/m_pedit.c (+14/-19)
tc/m_pedit.h (+3/-19)
tc/m_police.c (+7/-3)
tc/m_skbedit.c (+17/-2)
tc/m_tunnel_key.c (+232/-4)
tc/m_xt_old.c (+1/-2)
tc/p_eth.c (+2/-3)
tc/p_icmp.c (+2/-3)
tc/p_ip.c (+3/-4)
tc/p_ip6.c (+2/-3)
tc/p_tcp.c (+2/-3)
tc/p_udp.c (+2/-3)
tc/q_cake.c (+829/-0)
tc/q_choke.c (+1/-2)
tc/q_etf.c (+181/-0)
tc/q_fifo.c (+6/-4)
tc/q_fq.c (+28/-4)
tc/q_gred.c (+213/-37)
tc/q_htb.c (+1/-10)
tc/q_mqprio.c (+1/-2)
tc/q_netem.c (+114/-1)
tc/q_pie.c (+1/-1)
tc/q_red.c (+2/-12)
tc/q_sfq.c (+1/-2)
tc/q_skbprio.c (+84/-0)
tc/q_taprio.c (+400/-0)
tc/tc.c (+7/-9)
tc/tc_cbq.c (+1/-0)
tc/tc_class.c (+4/-5)
tc/tc_common.h (+15/-14)
tc/tc_core.c (+1/-0)
tc/tc_core.h (+0/-2)
tc/tc_estimator.c (+1/-0)
tc/tc_filter.c (+95/-40)
tc/tc_monitor.c (+9/-7)
tc/tc_qdisc.c (+2/-4)
tc/tc_red.c (+20/-0)
tc/tc_red.h (+5/-3)
tc/tc_util.c (+56/-69)
tc/tc_util.h (+0/-11)
testsuite/Makefile (+33/-16)
testsuite/lib/generic.sh (+14/-23)
testsuite/tests/ip/link/add_type_xfrm.t (+32/-0)
testsuite/tests/ip/route/add_default_route.t (+2/-0)
testsuite/tests/ip/tunnel/add_tunnel.t (+13/-0)
testsuite/tests/ss/ssfilter.t (+48/-0)
testsuite/tests/tc/batch.t (+23/-0)
testsuite/tests/tc/dsmark.t (+1/-1)
testsuite/tools/Makefile (+7/-1)
tipc/Makefile (+3/-1)
tipc/cmdl.c (+1/-1)
tipc/cmdl.h (+0/-2)
tipc/link.c (+273/-143)
tipc/nametable.c (+27/-9)
tipc/node.c (+3/-6)
tipc/tipc.c (+19/-1)
- Andreas Hasenack: Approve
- Canonical Server Core Reviewers: Pending requested
- Canonical Server: Pending requested
-
Diff: 160 lines (+138/-0)3 files modifieddebian/changelog (+8/-0)
debian/patches/series (+2/-0)
debian/patches/ss-review-ssfilter.patch (+128/-0)
- Andreas Hasenack: Approve
- Canonical Server: Pending requested
- Canonical Server Core Reviewers: Pending requested
-
Diff: 160 lines (+138/-0)3 files modifieddebian/changelog (+8/-0)
debian/patches/series (+2/-0)
debian/patches/ss-review-ssfilter.patch (+128/-0)
- Andreas Hasenack: Approve
- Canonical Server Core Reviewers: Pending requested
- Canonical Server: Pending requested
-
Diff: 160 lines (+138/-0)3 files modifieddebian/changelog (+8/-0)
debian/patches/series (+2/-0)
debian/patches/ss-review-ssfilter.patch (+128/-0)
https:/ /github. com/shemminger/ iproute2/ commit/ 38d209ecf2ae966 b9b25de4acb60cd ffb0e06ced
This commit looks like a fix for this issue:
ss: Review ssfilter
The original problem was ssfilter rejecting single expressions if
enclosed in braces, such as:
| sport = 22 or ( dport = 22 )
This is fixed by allowing 'expr' to be an 'exprlist' enclosed in braces.
The no longer required recursion in 'exprlist' being an 'exprlist'
enclosed in braces is dropped.
In addition to that, a few other things are changed:
* Remove pointless 'null' prefix in 'appled' before 'exprlist'.
* For simple equals matches, '=' operator was required for ports but not
allowed for hosts. Make this consistent by making '=' operator
optional in both cases.
Reported-by: Samuel Mannehed <email address hidden>
Fixes: b2038cc ("ssfilter: Eliminate shift/reduce conflicts")
Signed-off-by: Phil Sutter <email address hidden>
Signed-off-by: Stephen Hemminger <email address hidden>