* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: Yunsheng Lin @ 2018-11-10 2:02 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXeudFkHrJdCe3FCyNTxBO_YyvSb1rvSsbHBg1Yuy9m8g@mail.gmail.com>
On 2018/11/10 9:42, Cong Wang wrote:
> On Fri, Nov 9, 2018 at 5:39 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>
>> On 2018/11/10 3:43, Cong Wang wrote:
>>> Currently netdev_rx_csum_fault() only shows a device name,
>>> we need more information about the skb for debugging.
>>>
>>> Sample output:
>>>
>>> ens3: hw csum failure
>>> dev features: 0x0000000000014b89
>>> skb len=84 data_len=0 gso_size=0 gso_type=0 ip_summed=0 csum=0, csum_complete_sw=0, csum_valid=0
>>>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>> ---
>>> include/linux/netdevice.h | 5 +++--
>>> net/core/datagram.c | 6 +++---
>>> net/core/dev.c | 10 ++++++++--
>>> net/sunrpc/socklib.c | 2 +-
>>> 4 files changed, 15 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 857f8abf7b91..fabcd9fa6cf7 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -4332,9 +4332,10 @@ static inline bool can_checksum_protocol(netdev_features_t features,
>>> }
>>>
>>> #ifdef CONFIG_BUG
>>> -void netdev_rx_csum_fault(struct net_device *dev);
>>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb);
>>> #else
>>> -static inline void netdev_rx_csum_fault(struct net_device *dev)
>>> +static inline void netdev_rx_csum_fault(struct net_device *dev,
>>> + struct sk_buff *skb)
>>> {
>>> }
>>> #endif
>>> diff --git a/net/core/datagram.c b/net/core/datagram.c
>>> index 57f3a6fcfc1e..d8f4d55cd6c5 100644
>>> --- a/net/core/datagram.c
>>> +++ b/net/core/datagram.c
>>> @@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
>>> if (likely(!sum)) {
>>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>>> !skb->csum_complete_sw)
>>> - netdev_rx_csum_fault(skb->dev);
>>> + netdev_rx_csum_fault(skb->dev, skb);
>>> }
>>> if (!skb_shared(skb))
>>> skb->csum_valid = !sum;
>>> @@ -756,7 +756,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
>>> if (likely(!sum)) {
>>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>>> !skb->csum_complete_sw)
>>> - netdev_rx_csum_fault(skb->dev);
>>> + netdev_rx_csum_fault(skb->dev, skb);
>>> }
>>>
>>> if (!skb_shared(skb)) {
>>> @@ -810,7 +810,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
>>>
>>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>>> !skb->csum_complete_sw)
>>> - netdev_rx_csum_fault(NULL);
>>> + netdev_rx_csum_fault(NULL, skb);
>>> }
>>> return 0;
>>> fault:
>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>> index 0ffcbdd55fa9..2b337df26117 100644
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -3091,10 +3091,16 @@ EXPORT_SYMBOL(__skb_gso_segment);
>>>
>>> /* Take action when hardware reception checksum errors are detected. */
>>> #ifdef CONFIG_BUG
>>> -void netdev_rx_csum_fault(struct net_device *dev)
>>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
>>> {
>>> if (net_ratelimit()) {
>>> pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
>>> + if (dev)
>>> + pr_err("dev features: %pNF\n", &dev->features);
>>> + pr_err("skb len=%d data_len=%d gso_size=%d gso_type=%d ip_summed=%d csum=%x, csum_complete_sw=%d, csum_valid=%d\n",
>>> + skb->len, skb->data_len, skb_shinfo(skb)->gso_size,
>>> + skb_shinfo(skb)->gso_type, skb->ip_summed, skb->csum,
>>> + skb->csum_complete_sw, skb->csum_valid);
>>
>>
>> This function also have the netdev available, use netdev_err to log the error?
>
> It is apparently not me who picked pr_err() from the beginning,
> I just follow that pr_err(). If you are not happy with it, please send
> a followup.
Yes, but perhaps it is something to improve.
When using the netdev, then maybe it does not have to check if dev is null, because
netdev_err has handled the netdev being NULL case.
Maybe I missed something that netdev can not be used here?
If not, maybe I can send a followup.
>
>
>>
>> Also, dev->features was dumped before this patch, why remove it?
>
> Seriously? Where do I remove it? Please be specific. :)
Sorry, I missed that, I thought it was removed when adding the new log.
>
> .
>
^ permalink raw reply
* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-10 2:09 UTC (permalink / raw)
To: linyunsheng; +Cc: Linux Kernel Network Developers
In-Reply-To: <17be1af6-9aaf-593c-a68f-932d63aefbd5@huawei.com>
On Fri, Nov 9, 2018 at 6:02 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2018/11/10 9:42, Cong Wang wrote:
> > On Fri, Nov 9, 2018 at 5:39 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
> >>
> >> On 2018/11/10 3:43, Cong Wang wrote:
> >>> Currently netdev_rx_csum_fault() only shows a device name,
> >>> we need more information about the skb for debugging.
> >>>
> >>> Sample output:
> >>>
> >>> ens3: hw csum failure
> >>> dev features: 0x0000000000014b89
> >>> skb len=84 data_len=0 gso_size=0 gso_type=0 ip_summed=0 csum=0, csum_complete_sw=0, csum_valid=0
> >>>
> >>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> >>> ---
> >>> include/linux/netdevice.h | 5 +++--
> >>> net/core/datagram.c | 6 +++---
> >>> net/core/dev.c | 10 ++++++++--
> >>> net/sunrpc/socklib.c | 2 +-
> >>> 4 files changed, 15 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >>> index 857f8abf7b91..fabcd9fa6cf7 100644
> >>> --- a/include/linux/netdevice.h
> >>> +++ b/include/linux/netdevice.h
> >>> @@ -4332,9 +4332,10 @@ static inline bool can_checksum_protocol(netdev_features_t features,
> >>> }
> >>>
> >>> #ifdef CONFIG_BUG
> >>> -void netdev_rx_csum_fault(struct net_device *dev);
> >>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb);
> >>> #else
> >>> -static inline void netdev_rx_csum_fault(struct net_device *dev)
> >>> +static inline void netdev_rx_csum_fault(struct net_device *dev,
> >>> + struct sk_buff *skb)
> >>> {
> >>> }
> >>> #endif
> >>> diff --git a/net/core/datagram.c b/net/core/datagram.c
> >>> index 57f3a6fcfc1e..d8f4d55cd6c5 100644
> >>> --- a/net/core/datagram.c
> >>> +++ b/net/core/datagram.c
> >>> @@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
> >>> if (likely(!sum)) {
> >>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
> >>> !skb->csum_complete_sw)
> >>> - netdev_rx_csum_fault(skb->dev);
> >>> + netdev_rx_csum_fault(skb->dev, skb);
> >>> }
> >>> if (!skb_shared(skb))
> >>> skb->csum_valid = !sum;
> >>> @@ -756,7 +756,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
> >>> if (likely(!sum)) {
> >>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
> >>> !skb->csum_complete_sw)
> >>> - netdev_rx_csum_fault(skb->dev);
> >>> + netdev_rx_csum_fault(skb->dev, skb);
> >>> }
> >>>
> >>> if (!skb_shared(skb)) {
> >>> @@ -810,7 +810,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
> >>>
> >>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
> >>> !skb->csum_complete_sw)
> >>> - netdev_rx_csum_fault(NULL);
> >>> + netdev_rx_csum_fault(NULL, skb);
> >>> }
> >>> return 0;
> >>> fault:
> >>> diff --git a/net/core/dev.c b/net/core/dev.c
> >>> index 0ffcbdd55fa9..2b337df26117 100644
> >>> --- a/net/core/dev.c
> >>> +++ b/net/core/dev.c
> >>> @@ -3091,10 +3091,16 @@ EXPORT_SYMBOL(__skb_gso_segment);
> >>>
> >>> /* Take action when hardware reception checksum errors are detected. */
> >>> #ifdef CONFIG_BUG
> >>> -void netdev_rx_csum_fault(struct net_device *dev)
> >>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
> >>> {
> >>> if (net_ratelimit()) {
> >>> pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
> >>> + if (dev)
> >>> + pr_err("dev features: %pNF\n", &dev->features);
> >>> + pr_err("skb len=%d data_len=%d gso_size=%d gso_type=%d ip_summed=%d csum=%x, csum_complete_sw=%d, csum_valid=%d\n",
> >>> + skb->len, skb->data_len, skb_shinfo(skb)->gso_size,
> >>> + skb_shinfo(skb)->gso_type, skb->ip_summed, skb->csum,
> >>> + skb->csum_complete_sw, skb->csum_valid);
> >>
> >>
> >> This function also have the netdev available, use netdev_err to log the error?
> >
> > It is apparently not me who picked pr_err() from the beginning,
> > I just follow that pr_err(). If you are not happy with it, please send
> > a followup.
>
> Yes, but perhaps it is something to improve.
Sure, no one stops you from improving it in a followup patch. :)
> When using the netdev, then maybe it does not have to check if dev is null, because
> netdev_err has handled the netdev being NULL case.
> Maybe I missed something that netdev can not be used here?
> If not, maybe I can send a followup.
>
Maybe. Again, my patch intends to add a few debugging logs,
not to convert pr_err() to whatever else, they are totally different
goals. I choose pr_err() only because I follow the existing one,
not to say which one is better than the other.
Thanks.
^ permalink raw reply
* Re: [PATCH] add an initial version of snmp_counter.rst
From: Cong Wang @ 2018-11-10 2:20 UTC (permalink / raw)
To: peng yu; +Cc: Linux Kernel Network Developers, Randy Dunlap
In-Reply-To: <20181109181309.5541-1-yupeng0921@gmail.com>
(Cc Randy)
On Fri, Nov 9, 2018 at 10:13 AM yupeng <yupeng0921@gmail.com> wrote:
>
> The snmp_counter.rst run a set of simple experiments, explains the
> meaning of snmp counters depend on the experiments' results. This is
> an initial version, only covers a small part of the snmp counters.
I don't look into much details, so just a few high-level reviews:
1. Please try to group those counters by protocol, it would be easier
to search.
2. For many counters you provide a link to RFC, do you just copy
and paste them? Please try to expand.
3. _I think_ you don't need to show, for example, how to run a ping
command. It's safe to assume readers already know this. Therefore,
just explaining those counters is okay.
Thanks.
>
> Signed-off-by: yupeng <yupeng0921@gmail.com>
> ---
> Documentation/networking/index.rst | 1 +
> Documentation/networking/snmp_counter.rst | 963 ++++++++++++++++++++++
> 2 files changed, 964 insertions(+)
> create mode 100644 Documentation/networking/snmp_counter.rst
>
> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
> index bd89dae8d578..6a47629ef8ed 100644
> --- a/Documentation/networking/index.rst
> +++ b/Documentation/networking/index.rst
> @@ -31,6 +31,7 @@ Contents:
> net_failover
> alias
> bridge
> + snmp_counter
>
> .. only:: subproject
>
> diff --git a/Documentation/networking/snmp_counter.rst b/Documentation/networking/snmp_counter.rst
> new file mode 100644
> index 000000000000..2939c5acf675
> --- /dev/null
> +++ b/Documentation/networking/snmp_counter.rst
> @@ -0,0 +1,963 @@
> +====================
> +snmp counter tutorial
> +====================
> +
> +This document explains the meaning of snmp counters. For understanding
> +their meanings better, this document doesn't explain the counters one
> +by one, but creates a set of experiments, and explains the counters
> +depend on the experiments' results. The experiments are on one or two
> +virtual machines. Except for the test commands we use in the experiments,
> +the virtual machines have no other network traffic. We use the 'nstat'
> +command to get the values of snmp counters, before every test, we run
> +'nstat -n' to update the history, so the 'nstat' output would only
> +show the changes of the snmp counters. For more information about
> +nstat, please refer:
> +
> +http://man7.org/linux/man-pages/man8/nstat.8.html
> +
> +icmp ping
> +========
> +
> +Run the ping command against the public dns server 8.8.8.8::
> +
> + nstatuser@nstat-a:~$ ping 8.8.8.8 -c 1
> + PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
> + 64 bytes from 8.8.8.8: icmp_seq=1 ttl=119 time=17.8 ms
> +
> + --- 8.8.8.8 ping statistics ---
> + 1 packets transmitted, 1 received, 0% packet loss, time 0ms
> + rtt min/avg/max/mdev = 17.875/17.875/17.875/0.000 ms
> +
> +The nstayt result::
> +
> + nstatuser@nstat-a:~$ nstat
> + #kernel
> + IpInReceives 1 0.0
> + IpInDelivers 1 0.0
> + IpOutRequests 1 0.0
> + IcmpInMsgs 1 0.0
> + IcmpInEchoReps 1 0.0
> + IcmpOutMsgs 1 0.0
> + IcmpOutEchos 1 0.0
> + IcmpMsgInType0 1 0.0
> + IcmpMsgOutType8 1 0.0
> + IpExtInOctets 84 0.0
> + IpExtOutOctets 84 0.0
> + IpExtInNoECTPkts 1 0.0
> +
> +The nstat output could be divided into two part: one with the 'Ext'
> +keyword, another without the 'Ext' keyword. If the counter name
> +doesn't have 'Ext', it is defined by one of snmp rfc, if it has 'Ext',
> +it is a kernel extent counter. Below we explain them one by one.
> +
> +The rfc defined counters
> +----------------------
> +
> +* IpInReceives
> +The total number of input datagrams received from interfaces,
> +including those received in error.
> +
> +https://tools.ietf.org/html/rfc1213#page-26
> +
> +* IpInDelivers
> +The total number of input datagrams successfully delivered to IP
> +user-protocols (including ICMP).
> +
> +https://tools.ietf.org/html/rfc1213#page-28
> +
> +* IpOutRequests
> +The total number of IP datagrams which local IP user-protocols
> +(including ICMP) supplied to IP in requests for transmission. Note
> +that this counter does not include any datagrams counted in
> +ipForwDatagrams.
> +
> +https://tools.ietf.org/html/rfc1213#page-28
> +
> +* IcmpInMsgs
> +The total number of ICMP messages which the entity received. Note
> +that this counter includes all those counted by icmpInErrors.
> +
> +https://tools.ietf.org/html/rfc1213#page-41
> +
> +* IcmpInEchoReps
> +The number of ICMP Echo Reply messages received.
> +
> +https://tools.ietf.org/html/rfc1213#page-42
> +
> +* IcmpOutMsgs
> +The total number of ICMP messages which this entity attempted to send.
> +Note that this counter includes all those counted by icmpOutErrors.
> +
> +https://tools.ietf.org/html/rfc1213#page-43
> +
> +* IcmpOutEchos
> +The number of ICMP Echo (request) messages sent.
> +
> +https://tools.ietf.org/html/rfc1213#page-45
> +
> +IcmpMsgInType0 and IcmpMsgOutType8 are not defined by any snmp related
> +RFCs, but their meaning are quite straightforward, they count the
> +packet number of specific icmp packet types. We could find the icmp
> +types here:
> +
> +https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml
> +
> +Type 8 is echo, type 0 is echo reply.
> +
> +Until now, we can easily explain these items of the nstat: We sent an
> +icmp echo request, so IpOutRequests, IcmpOutMsgs, IcmpOutEchos and
> +IcmpMsgOutType8 were increased 1. We got icmp echo reply from 8.8.8.8,
> +so IpInReceives, IcmpInMsgs, IcmpInEchoReps, IcmpMsgInType0 were
> +increased 1. The icmp echo reply was passed to icmp layer via ip
> +layer, so IpInDelivers was increased 1.
> +
> +Please note, these metrics don't aware LRO/GRO, e.g., IpOutRequests
> +might count 1 packet, but hardware splits it to 2, and sends them
> +separately.
> +
> +IpExtInOctets and IpExtOutOctets
> +------------------------------
> +They are linux kernel extensions, no rfc definitions. Please note,
> +rfc1213 indeed defines ifInOctets and ifOutOctets, but they
> +are different things. The ifInOctets and ifOutOctets are packets
> +size which includes the mac layer. But IpExtInOctets and IpExtOutOctets
> +are only ip layer sizes.
> +
> +In our example, an ICMP echo request has four parts:
> +* 14 bytes mac header
> +* 20 bytes ip header
> +* 16 bytes icmp header
> +* 48 bytes data (default value of the ping command)
> +
> +So IpExtInOctets value is 20+16+48=84. The IpExtOutOctets is similar.
> +
> +IpExtInNoECTPkts
> +---------------
> +We could find IpExtInNoECTPkts in the nstat output, but kernel provide
> +four similar counters, we explain them together, they are:
> +* IpExtInNoECTPkts
> +* IpExtInECT1Pkts
> +* IpExtInECT0Pkts
> +* IpExtInCEPkts
> +
> +They indicate four kinds of ECN IP packets, they are defined here:
> +
> +https://tools.ietf.org/html/rfc3168#page-6
> +
> +These 4 counters calculate how many packets received per ECN
> +status. They count the real frame number regardless the LRO/GRO. So
> +for the same packet, you might find that IpInReceives count 1, but
> +IpExtInNoECTPkts counts 2 or more.
> +
> +additional explain
> +-----------------
> +The ip layer counters are recorded by the ip layer code in the kernel. I mean, if you send a packet to a lower layer directly, Linux
> +kernel won't record it. For example, tcpreplay will open an
> +AF_PACKET socket, and send the packet to layer 2, although it could send
> +an IP packet, you can't find it from the nstat output. Here is an
> +example:
> +
> +We capture the ping packet by tcpdump::
> +
> + nstatuser@nstat-a:~$ sudo tcpdump -w /tmp/ping.pcap dst 8.8.8.8
> +
> +Then run ping command::
> +
> + nstatuser@nstat-a:~$ ping 8.8.8.8 -c 1
> +
> +Terminate tcpdump by Ctrl-C, and run 'nstat -n' to update the nstat
> +history. Then run tcpreplay::
> +
> + nstatuser@nstat-a:~$ sudo tcpreplay --intf1=ens3 /tmp/ping.pcap
> + Actual: 1 packets (98 bytes) sent in 0.000278 seconds
> + Rated: 352517.9 Bps, 2.82 Mbps, 3597.12 pps
> + Flows: 1 flows, 3597.12 fps, 1 flow packets, 0 non-flow
> + Statistics for network device: ens3
> + Successful packets: 1
> + Failed packets: 0
> + Truncated packets: 0
> + Retried packets (ENOBUFS): 0
> + Retried packets (EAGAIN): 0
> +
> +Check the nstat output::
> +
> + nstatuser@nstat-a:~$ nstat
> + #kernel
> + IpInReceives 1 0.0
> + IpInDelivers 1 0.0
> + IcmpInMsgs 1 0.0
> + IcmpInEchoReps 1 0.0
> + IcmpMsgInType0 1 0.0
> + IpExtInOctets 84 0.0
> + IpExtInNoECTPkts 1 0.0
> +
> +We can see, nstat only show the received packet, because the IP layer
> +of kernel only know the reply of 8.8.8.8, it doesn't know what
> +tcpreplay sent.
> +
> +At the same time, when you use AF_INET socket, even you use the
> +SOCK_RAW option, the IP layer will still try to verify whether the
> +packet is an ICMP packet, if it is, kernel will still count it to its
> +counters and you can find it in the output of nstat.
> +
> +tcp 3 way handshake
> +==================
> +
> +On server side, we run::
> +
> + nstatuser@nstat-b:~$ nc -lknv 0.0.0.0 9000
> + Listening on [0.0.0.0] (family 0, port 9000)
> +
> +On client side, we run::
> +
> + nstatuser@nstat-a:~$ nc -nv 192.168.122.251 9000
> + Connection to 192.168.122.251 9000 port [tcp/*] succeeded!
> +
> +The server listened on tcp 9000 port, the client connected to it, they
> +completed the 3-way handshake.
> +
> +On server side, we can find below nstat output::
> +
> + nstatuser@nstat-b:~$ nstat | grep -i tcp
> + TcpPassiveOpens 1 0.0
> + TcpInSegs 2 0.0
> + TcpOutSegs 1 0.0
> + TcpExtTCPPureAcks 1 0.0
> +
> +On client side, we can find below nstat output::
> +
> + nstatuser@nstat-a:~$ nstat | grep -i tcp
> + TcpActiveOpens 1 0.0
> + TcpInSegs 1 0.0
> + TcpOutSegs 2 0.0
> +
> +Except for TcpExtTCPPureAcks, all other counters are defined by rfc1213
> +
> +* TcpActiveOpens
> +The number of times TCP connections have made a direct transition to
> +the SYN-SENT state from the CLOSED state.
> +
> +https://tools.ietf.org/html/rfc1213#page-47
> +
> +* TcpPassiveOpens
> +The number of times TCP connections have made a direct transition to
> +the SYN-RCVD state from the LISTEN state.
> +
> +https://tools.ietf.org/html/rfc1213#page-47
> +
> +* TcpInSegs
> +The total number of segments received, including those received in
> +error. This count includes segments received on currently established
> +connections.
> +
> +https://tools.ietf.org/html/rfc1213#page-48
> +
> +* TcpOutSegs
> +The total number of segments sent, including those on current
> +connections but excluding those containing only retransmitted octets.
> +
> +https://tools.ietf.org/html/rfc1213#page-48
> +
> +
> +The TcpExtTCPPureAcks is an extension in linux kernel. When kernel
> +receives a TCP packet which set ACK flag and with no data, either
> +TcpExtTCPPureAcks or TcpExtTCPHPAcks will increase 1. We will discuss
> +it in a later section.
> +
> +Now we can easily explain the nstat outputs on the server side and client
> +side.
> +
> +When the server received the first syn, it replied a syn+ack, and came into
> +SYN-RCVD state, so TcpPassiveOpens increased 1. The server received
> +syn, sent syn+ack, received ack, so server sent 1 packet, received 2
> +packets, TcpInSegs increased 2, TcpOutSegs increased 1. The last ack
> +of the 3-way handshake is a pure ack without data, so
> +TcpExtTCPPureAcks increased 1.
> +
> +When the client sent syn, the client came into the SYN-SENT state, so
> +TcpActiveOpens increased 1, client sent syn, received syn+ack, sent
> +ack, so client sent 2 packets, received 1 packet, TcpInSegs increased
> +1, TcpOutSegs increased 2.
> +
> +Note: about TcpInSegs and TcpOutSegs, rfc1213 doesn't define the
> +behaviors when gso/gro/tso are enabled on the NIC (network interface
> +card). On current linux implementation, TcpOutSegs awares gso/tso, but
> +TcpInSegs doesn't aware gro. So TcpOutSegs will count the actual
> +packet number even only 1 packet is sent via tcp layer. If multiple
> +packets arrived at a NIC, and they are merged into 1 packet, TcpInSegs
> +will only count 1.
> +
> +tcp disconnect
> +=============
> +
> +Continue our previous example, on the server side, we have run::
> +
> + nstatuser@nstat-b:~$ nc -lknv 0.0.0.0 9000
> + Listening on [0.0.0.0] (family 0, port 9000)
> +
> +On client side, we have run::
> +
> + nstatuser@nstat-a:~$ nc -nv 192.168.122.251 9000
> + Connection to 192.168.122.251 9000 port [tcp/*] succeeded!
> +
> +Now we type Ctrl-C on the client side, stop the tcp connection between the
> +two nc command. Then we check the nstat output.
> +
> +On server side::
> +
> + nstatuser@nstat-b:~$ nstat | grep -i tcp
> + TcpInSegs 2 0.0
> + TcpOutSegs 1 0.0
> + TcpExtTCPPureAcks 1 0.0
> + TcpExtTCPOrigDataSent 1 0.0
> +
> +On client side::
> +
> + nstatuser@nstat-b:~$ nstat | grep -i tcp
> + TcpInSegs 2 0.0
> + TcpOutSegs 1 0.0
> + TcpExtTCPPureAcks 1 0.0
> + TcpExtTCPOrigDataSent 1 0.0
> +
> +Wait for more than 1 minute, run nstat on client again::
> +
> + nstatuser@nstat-a:~$ nstat | grep -i tcp
> + TcpExtTW 1 0.0
> +
> +Most of the counters are explained in the previous section except
> +two: TcpExtTCPOrigDataSent and TcpExtTW. Both of them are linux kernel
> +extensions.
> +
> +TcpExtTW means a tcp connection is closed normally via
> +time wait stage, not via tcp reuse process.
> +
> +About TcpExtTCPOrigDataSent, Below kernel patch has a good explanation:
> +
> +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f19c29e3e391a66a273e9afebaf01917245148cd
> +
> +I pasted it here::
> +
> + TCPOrigDataSent: number of outgoing packets with original data
> + (excluding retransmission but including data-in-SYN). This counter is
> + different from TcpOutSegs because TcpOutSegs also tracks pure
> + ACKs. TCPOrigDataSent is more useful to track the TCP retransmission rate.
> +
> +the effect of gso and gro
> +=======================
> +
> +The Generic Segmentation Offload (GSO) and Generic Receive Offload
> +would affect the metrics of the packet in/out on both ip and tcp
> +layer. Here is an iperf example. Before the test, run below command to
> +make sure both gso and gro are enabled on the NIC::
> +
> + $ sudo ethtool -k ens3 | egrep '(generic-segmentation-offload|generic-receive-offload)'
> + generic-segmentation-offload: on
> + generic-receive-offload: on
> +
> +On server side, run::
> +
> + iperf3 -s -p 9000
> +
> +On client side, run::
> +
> + iperf3 -c 192.168.122.251 -p 9000 -t 5 -P 10
> +
> +The server listened on tcp port 9000, the client connected to the server,
> +created 10 threads parallel, run 5 seconds. After the pierf3 stopped, we
> +run nstat on both the server and client.
> +
> +On server side::
> +
> + nstatuser@nstat-b:~$ nstat
> + #kernel
> + IpInReceives 36346 0.0
> + IpInDelivers 36346 0.0
> + IpOutRequests 33836 0.0
> + TcpPassiveOpens 11 0.0
> + TcpEstabResets 2 0.0
> + TcpInSegs 36346 0.0
> + TcpOutSegs 33836 0.0
> + TcpOutRsts 20 0.0
> + TcpExtDelayedACKs 26 0.0
> + TcpExtTCPHPHits 32120 0.0
> + TcpExtTCPPureAcks 16 0.0
> + TcpExtTCPHPAcks 5 0.0
> + TcpExtTCPAbortOnData 5 0.0
> + TcpExtTCPAbortOnClose 2 0.0
> + TcpExtTCPRcvCoalesce 7306 0.0
> + TcpExtTCPOFOQueue 1354 0.0
> + TcpExtTCPOrigDataSent 15 0.0
> + IpExtInOctets 311732432 0.0
> + IpExtOutOctets 1785119 0.0
> + IpExtInNoECTPkts 214032 0.0
> +
> +Client side::
> +
> + nstatuser@nstat-a:~$ nstat
> + #kernel
> + IpInReceives 33836 0.0
> + IpInDelivers 33836 0.0
> + IpOutRequests 43786 0.0
> + TcpActiveOpens 11 0.0
> + TcpEstabResets 10 0.0
> + TcpInSegs 33836 0.0
> + TcpOutSegs 214072 0.0
> + TcpRetransSegs 3876 0.0
> + TcpExtDelayedACKs 7 0.0
> + TcpExtTCPHPHits 5 0.0
> + TcpExtTCPPureAcks 2719 0.0
> + TcpExtTCPHPAcks 31071 0.0
> + TcpExtTCPSackRecovery 607 0.0
> + TcpExtTCPSACKReorder 61 0.0
> + TcpExtTCPLostRetransmit 90 0.0
> + TcpExtTCPFastRetrans 3806 0.0
> + TcpExtTCPSlowStartRetrans 62 0.0
> + TcpExtTCPLossProbes 38 0.0
> + TcpExtTCPSackRecoveryFail 8 0.0
> + TcpExtTCPSackShifted 203 0.0
> + TcpExtTCPSackMerged 778 0.0
> + TcpExtTCPSackShiftFallback 700 0.0
> + TcpExtTCPSpuriousRtxHostQueues 4 0.0
> + TcpExtTCPAutoCorking 14 0.0
> + TcpExtTCPOrigDataSent 214038 0.0
> + TcpExtTCPHystartTrainDetect 8 0.0
> + TcpExtTCPHystartTrainCwnd 172 0.0
> + IpExtInOctets 1785227 0.0
> + IpExtOutOctets 317789680 0.0
> + IpExtInNoECTPkts 33836 0.0
> +
> +The TcpOutSegs and IpOutRequests on the server are 33836, exactly the
> +same as IpExtInNoECTPkts, IpInReceives, IpInDelivers and TcpInSegs on
> +the client side. During iperf3 test, the server only reply very short
> +packets, so gso and gro has no effect on the server's reply.
> +
> +On the client side, TcpOutSegs is 214072, IpOutRequests is 43786, the
> +tcp layer packet out is larger than ip layer packet out, because
> +TcpOutSegs count the packet number after gso, but IpOutRequests
> +doesn't. On the server side, IpExtInNoECTPkts is 214032, this number
> +is smaller a little than the TcpOutSegs on the client side (214072), it
> +might cause by the packet loss. The IpInReceives, IpInDelivers and
> +TcpInSegs are obviously smaller than the TcpOutSegs on the client side,
> +because these counters calculate the packet after gro.
> +
> +tcp counters in established state
> +================================
> +
> +Run nc on server::
> +
> + nstatuser@nstat-b:~$ nc -lkv 0.0.0.0 9000
> + Listening on [0.0.0.0] (family 0, port 9000)
> +
> +Run nc on client:
> +
> + nstatuser@nstat-a:~$ nc -v nstat-b 9000
> + Connection to nstat-b 9000 port [tcp/*] succeeded!
> +
> +Input a string in the nc client ('hello' in our example):
> +
> + nstatuser@nstat-a:~$ nc -v nstat-b 9000
> + Connection to nstat-b 9000 port [tcp/*] succeeded!
> + hello
> +
> +The client side nstat output:
> +
> + nstatuser@nstat-a:~$ nstat
> + #kernel
> + IpInReceives 1 0.0
> + IpInDelivers 1 0.0
> + IpOutRequests 1 0.0
> + TcpInSegs 1 0.0
> + TcpOutSegs 1 0.0
> + TcpExtTCPPureAcks 1 0.0
> + TcpExtTCPOrigDataSent 1 0.0
> + IpExtInOctets 52 0.0
> + IpExtOutOctets 58 0.0
> + IpExtInNoECTPkts 1 0.0
> +
> +The server side nstat output:
> +
> + nstatuser@nstat-b:~$ nstat
> + #kernel
> + IpInReceives 1 0.0
> + IpInDelivers 1 0.0
> + IpOutRequests 1 0.0
> + TcpInSegs 1 0.0
> + TcpOutSegs 1 0.0
> + IpExtInOctets 58 0.0
> + IpExtOutOctets 52 0.0
> + IpExtInNoECTPkts 1 0.0
> +
> +Input a string in nc client side again ('world' in our exmaple):
> +
> + nstatuser@nstat-a:~$ nc -v nstat-b 9000
> + Connection to nstat-b 9000 port [tcp/*] succeeded!
> + hello
> + world
> +
> +Client side nstat output:
> +
> + nstatuser@nstat-a:~$ nstat
> + #kernel
> + IpInReceives 1 0.0
> + IpInDelivers 1 0.0
> + IpOutRequests 1 0.0
> + TcpInSegs 1 0.0
> + TcpOutSegs 1 0.0
> + TcpExtTCPHPAcks 1 0.0
> + TcpExtTCPOrigDataSent 1 0.0
> + IpExtInOctets 52 0.0
> + IpExtOutOctets 58 0.0
> + IpExtInNoECTPkts 1 0.0
> +
> +
> +Server side nstat output:
> +
> + nstatuser@nstat-b:~$ nstat
> + #kernel
> + IpInReceives 1 0.0
> + IpInDelivers 1 0.0
> + IpOutRequests 1 0.0
> + TcpInSegs 1 0.0
> + TcpOutSegs 1 0.0
> + TcpExtTCPHPHits 1 0.0
> + IpExtInOctets 58 0.0
> + IpExtOutOctets 52 0.0
> + IpExtInNoECTPkts 1 0.0
> +
> +Compare the first client side output and the second client side
> +output, we could find one difference: the first one had a
> +'TcpExtTCPPureAcks', but the second one had a
> +'TcpExtTCPHPAcks'. The first server side output and the second server
> +side output had a difference too: the second server side output had a
> +TcpExtTCPHPHits, but the first server side output didn't have it. The
> +network traffic patterns were exactly the same: the client sent a packet to the server, the server replied an ack. But kernel handled them in different
> +ways. When kernel receives a tpc packet in the established status,
> +kernel has two paths to handle the packet, one is fast path, another
> +is slow path. The comment in kernel code provides a good explanation of
> +them, I paste them below:
> +
> + It is split into a fast path and a slow path. The fast path is
> + disabled when:
> + - A zero window was announced from us - zero window probing
> + is only handled properly on the slow path.
> + - Out of order segments arrived.
> + - Urgent data is expected.
> + - There is no buffer space left
> + - Unexpected TCP flags/window values/header lengths are received
> + (detected by checking the TCP header against pred_flags)
> + - Data is sent in both directions. The fast path only supports pure senders
> + or pure receivers (this means either the sequence number or the ack
> + value must stay constant)
> + - Unexpected TCP option.
> +
> +Kernel will try to use fast path unless any of the above conditions
> +are satisfied. If the packets are out of order, kernel will handle
> +them in slow path, which means the performance might be not very
> +good. Kernel would also come into slow path if the "Delayed ack" is
> +used, because when using "Delayed ack", the data is sent in both
> +directions. When the tcp window scale option is not used, kernel will
> +try to enable fast path immediately when the connection comes into the established
> +state, but if the tcp window scale option is used, kernel will disable
> +the fast path at first, and try to enable it after kerenl receives
> +packets. We could use the 'ss' command to verify whether the window
> +scale option is used. e.g. run below command on either server or
> +client:
> +
> + nstatuser@nstat-a:~$ ss -o state established -i '( dport = :9000 or sport = :9000 )
> + Netid Recv-Q Send-Q Local Address:Port Peer Address:Port
> + tcp 0 0 192.168.122.250:40654 192.168.122.251:9000
> + ts sack cubic wscale:7,7 rto:204 rtt:0.98/0.49 mss:1448 pmtu:1500 rcvmss:536 advmss:1448 cwnd:10 bytes_acked:1 segs_out:2 segs_in:1 send 118.2Mbps lastsnd:46572 lastrcv:46572 lastack:46572 pacing_rate 236.4Mbps rcv_space:29200 rcv_ssthresh:29200 minrtt:0.98
> +
> +The 'wscale:7,7' means both server and client set the window scale
> +option to 7. Now we could explain the nstat output in our test:
> +
> +In the first nstat output of client side, the client sent a packet, server
> +reply an ack, when kernel handled this ack, the fast path was not
> +enabled, so the ack was counted into 'TcpExtTCPPureAcks'.
> +In the second nstat output of client side, the client sent a packet again,
> +and received another ack from the server, this time, the fast path is
> +enabled, and the ack was qualified for fast path, so it was handled by
> +the fast path, so this ack was counted into TcpExtTCPHPAcks.
> +In the first nstat output of server side, the fast path was not enabled,
> +so there was no 'TcpExtTCPHPHits'.
> +In the second nstat output of server side, the fast path was enabled,
> +and the packet received from client qualified for fast path, so it
> +was counted into 'TcpExtTCPHPHits'.
> +
> +tcp abort
> +========
> +
> +Some counters indicate the reaons why tcp layer want to send a rst,
> +they are:
> +* TcpExtTCPAbortOnData
> +* TcpExtTCPAbortOnClose
> +* TcpExtTCPAbortOnMemory
> +* TcpExtTCPAbortOnTimeout
> +* TcpExtTCPAbortOnLinger
> +* TcpExtTCPAbortFailed
> +
> +TcpExtTCPAbortOnData
> +-------------------
> +
> +It means tcp layer has data in flight, but need to close the
> +connection. So tcp layer sends a rst to the other side, indicate the
> +connection is not closed very graceful. An easy way to increase this
> +counter is using the SO_LINGER option. Please refer to the SO_LINGER
> +section of the socket man page:
> +
> +http://man7.org/linux/man-pages/man7/socket.7.html).
> +
> +By default, when an application closes a connection, the close function
> +will return immediately and kernel will try to send the in-flight data
> +async. If you use the SO_LINGER option, set l_onoff to 1, and l_linger
> +to a positive number, the close function won't return immediately, but
> +wait for the in-flight data are acked by the other side, the max wait
> +time is l_linger seconds. If set l_onoff to 1 and set l_linger to 0,
> +when the application closes a connection, kernel will send an rst
> +immediately, and increase the TcpExtTCPAbortOnData counter.
> +
> +We run nc on the server side::
> +
> + nstatuser@nstat-b:~$ nc -lkv 0.0.0.0 9000
> + Listening on [0.0.0.0] (family 0, port 9000)
> +
> +Run below python code on the client side::
> +
> + import socket
> + import struct
> +
> + server = 'nstat-b' # server address
> + port = 9000
> +
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 0))
> + s.connect((server, port))
> + s.close()
> +
> +On client side, we could see TcpExtTCPAbortOnData increased::
> +
> + nstatuser@nstat-a:~$ nstat | grep -i abort
> + TcpExtTCPAbortOnData 1 0.0
> +
> +If we capture packet by tcpdump, we could see the client send rst
> +instead of fin.
> +
> +
> +TcpExtTCPAbortOnClose
> +--------------------
> +
> +This counter means the tcp layer has unread data when an application
> +want to close a connection.
> +
> +On the server side, we run below python script:
> +
> + import socket
> + import time
> +
> + port = 9000
> +
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.bind(('0.0.0.0', port))
> + s.listen(1)
> + sock, addr = s.accept()
> + while True:
> + time.sleep(9999999)
> +
> +This python script listen on 9000 port, but doesn't read anything from
> +the connection.
> +
> +On the client side, we send the string "hello" by nc:
> +
> + nstatuser@nstat-a:~$ echo "hello" | nc nstat-b 9000
> +
> +Then, we come back to the server side, the server has received the "hello"
> +packet, and tcp layer has acked this packet, but the application didn't
> +read it yet. We type Ctrl-C to terminate the server script. Then we
> +could find TcpExtTCPAbortOnClose increased 1 on the server side:
> +
> + nstatuser@nstat-b:~$ nstat | grep -i abort
> + TcpExtTCPAbortOnClose 1 0.0
> +
> +If we run tcpdump on the server side, we could find the server sent a
> +rst after we type Ctrl-C.
> +
> +TcpExtTCPAbortOnMemory
> +--------------------
> +
> +When an application closes a tcp connection, kernel still need to track
> +the connection, let it complete the tcp disconnect process. E.g. an
> +app calls the close method of a socket, kernel sends fin to the other
> +side of the connection, then the app has no relationship with the
> +socket any more, but kernel need to keep the socket, this socket
> +becomes an orphan socket, kernel waits for the reply of the other side,
> +and would come to the TIME_WAIT state finally. When kernel has no
> +enough memory to keep the orphan socket, kernel would send an rst to
> +the other side, and delete the socket, in such situation, kernel will
> +increase 1 to the TcpExtTCPAbortOnMemory. Two conditions would trigger
> +TcpExtTCPAbortOnMemory:
> +
> +* the memory used by tcp protocol is higher than the third value of
> +the tcp_mem. Please refer the tcp_mem section in the tcp man page:
> +
> +http://man7.org/linux/man-pages/man7/tcp.7.html
> +
> +* the orphan socket count is higher than net.ipv4.tcp_max_orphans
> +
> +Below is an example which let the orphan socket count be higher than
> +net.ipv4.tcp_max_orphans.
> +
> +Change tcp_max_orphans to a smaller value on client::
> +
> + sudo bash -c "echo 10 > /proc/sys/net/ipv4/tcp_max_orphans"
> +
> +Client code (create 64 connection to server)::
> +
> + nstatuser@nstat-a:~$ cat client_orphan.py
> + import socket
> + import time
> +
> + server = 'nstat-b' # server address
> + port = 9000
> +
> + count = 64
> +
> + connection_list = []
> +
> + for i in range(64):
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.connect((server, port))
> + connection_list.append(s)
> + print("connection_count: %d" % len(connection_list))
> +
> + while True:
> + time.sleep(99999)
> +
> +Server code (accept 64 connection from client)::
> +
> + nstatuser@nstat-b:~$ cat server_orphan.py
> + import socket
> + import time
> +
> + port = 9000
> + count = 64
> +
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.bind(('0.0.0.0', port))
> + s.listen(count)
> + connection_list = []
> + while True:
> + sock, addr = s.accept()
> + connection_list.append((sock, addr))
> + print("connection_count: %d" % len(connection_list))
> +
> +Run the python scripts on server and client.
> +
> +On server::
> +
> + python3 server_orphan.py
> +
> +On client::
> +
> + python3 client_orphan.py
> +
> +Run iptables on server::
> +
> + sudo iptables -A INPUT -i ens3 -p tcp --destination-port 9000 -j DROP
> +
> +Type Ctrl-C on client, stop client_orphan.py.
> +
> +Check TcpExtTCPAbortOnMemory on client::
> +
> + nstatuser@nstat-a:~$ nstat | grep -i abort
> + TcpExtTCPAbortOnMemory 54 0.0
> +
> +Check orphane socket count on client::
> +
> + nstatuser@nstat-a:~$ ss -s
> + Total: 131 (kernel 0)
> + TCP: 14 (estab 1, closed 0, orphaned 10, synrecv 0, timewait 0/0), ports 0
> +
> + Transport Total IP IPv6
> + * 0 - -
> + RAW 1 0 1
> + UDP 1 1 0
> + TCP 14 13 1
> + INET 16 14 2
> + FRAG 0 0 0
> +
> +The explanation of the test: after run server_orphan.py and
> +client_orphan.py, we set up 64 connections between server and
> +client. Run the iptables command, the server will drop all packets from
> +the client, type Ctrl-C on client_orphan.py, the system of the client
> +would try to close these connections, and before they are closed
> +gracefully, these connections became orphan sockets. As the iptables
> +of the server blocked packets from the client, the server won't receive fin
> +from the client, so all connection on clients would be stuck on FIN_WAIT_1
> +stage, so they will keep as orphan sockets until timeout. We have echo
> +10 to /proc/sys/net/ipv4/tcp_max_orphans, so the client system would
> +only keep 10 orphan sockets, for all other orphan sockets, the client
> +system sent rst for them and delete them. We have 64 connections, so
> +the 'ss -s' command shows the system has 10 orphan sockets, and the
> +value of TcpExtTCPAbortOnMemory was 54.
> +
> +An additional explanation about orphan socket count: You could find the
> +exactly orphan socket count by the 'ss -s' command, but when kernel
> +decide whither increases TcpExtTCPAbortOnMemory and sends rst, kernel
> +doesn't always check the exactly orphan socket count. For increasing
> +performance, kernel checks an approximate count firstly, if the
> +approximate count is more than tcp_max_orphans, kernel checks the
> +exact count again. So if the approximate count is less than
> +tcp_max_orphans, but exactly count is more than tcp_max_orphans, you
> +would find TcpExtTCPAbortOnMemory is not increased at all. If
> +tcp_max_orphans is large enough, it won't occur, but if you decrease
> +tcp_max_orphans to a small value like our test, you might find this
> +issue. So in our test, the client set up 64 connections although the
> +tcp_max_orphans is 10. If the client only set up 11 connections, we
> +can't find the change of TcpExtTCPAbortOnMemory.
> +
> +TcpExtTCPAbortOnTimeout
> +----------------------
> +This counter will increase when any of the tcp timers expire. In this
> +situation, kernel won't send rst, just give up the connection.
> +Continue the previous test, we wait for several minutes, because the
> +iptables on the server blocked the traffic, the server wouldn't receive
> +fin, and all the client's orphan sockets would timeout on the
> +FIN_WAIT_1 state finally. So we wait for a few minutes, we could find
> +10 timeout on the client::
> +
> + nstatuser@nstat-a:~$ nstat | grep -i abort
> + TcpExtTCPAbortOnTimeout 10 0.0
> +
> +TcpExtTCPAbortOnLinger
> +---------------------
> +When a tcp connection comes into FIN_WAIT_2 state, instead of waiting
> +for the fin packet from the other side, kernel could send a rst and
> +delete the socket immediately. This is not the default behavior of
> +linux kernel tcp stack, but after configuring socket option, you could
> +let kernel follow this behavior. Below is an example.
> +
> +The server side code::
> +
> + nstatuser@nstat-b:~$ cat server_linger.py
> + import socket
> + import time
> +
> + port = 9000
> +
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.bind(('0.0.0.0', port))
> + s.listen(1)
> + sock, addr = s.accept()
> + while True:
> + time.sleep(9999999)
> +
> +The client side code::
> +
> + nstatuser@nstat-a:~$ cat client_linger.py
> + import socket
> + import struct
> +
> + server = 'nstat-b' # server address
> + port = 9000
> +
> + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> + s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack('ii', 1, 10))
> + s.setsockopt(socket.SOL_TCP, socket.TCP_LINGER2, struct.pack('i', -1))
> + s.connect((server, port))
> + s.close()
> +
> +Run server_linger.py on server::
> +
> + nstatuser@nstat-b:~$ python3 server_linger.py
> +
> +Run client_linger.py on client::
> +
> + nstatuser@nstat-a:~$ python3 client_linger.py
> +
> +After run client_linger.py, check the output of nstat::
> +
> + nstatuser@nstat-a:~$ nstat | grep -i abort
> + TcpExtTCPAbortOnLinger 1 0.0
> +
> +TcpExtTCPAbortFailed
> +-------------------
> +The kernel tcp layer will send rst if the RFC 2525 2.17 section is satisfied:
> +
> +https://tools.ietf.org/html/rfc2525#page-50
> +
> +If an internal error occurs during this process, TcpExtTCPAbortFailed
> +will be increased.
> +
> +TcpExtListenOverflows and TcpExtListenDrops
> +========================================
> +When kernel receive a syn from a client, and if the tcp accept queue
> +is full, kernel will drop the syn and add 1 to TcpExtListenOverflows.
> +At the same time kernel will also add 1 to TcpExtListenDrops. When
> +a tcp socket is in LISTEN state, and kernel need to drop a packet,
> +kernel would always add 1 to TcpExtListenDrops. So increase
> +TcpExtListenOverflows would let TcpExtListenDrops increasing at the
> +same time, but TcpExtListenDrops would also increase without
> +TcpExtListenOverflows increasing, e.g. a memory allocation fail would
> +also let TcpExtListenDrops increase.
> +
> +Note: The above explain bases on kernel 4.15 or above version, on an
> +old kernel, the tcp stack has different behavior when tcp accept queue
> +is full. On the old kernel, tcp stack won't drop the syn, it would
> +complete the 3-way handshake, but as the accept queue is full, tcp
> +stack will keep the socket in the tcp half-open queue. As it is in the
> +half open queue, tcp stack will send syn+ack on an exponential backoff
> +timer, after client replies ack, tcp stack checks whether the accept
> +queue is still full, if it is not full, move the socket to accept
> +queue, if it is full, keeps the socket in the half-open queue, at next
> +time client replies ack, this socket will get another chance to move
> +to the accept queue.
> +
> +Here is an example:
> +
> +On server, run the nc command, listen on port 9000::
> +
> + nstatuser@nstat-b:~$ nc -lkv 0.0.0.0 9000
> + Listening on [0.0.0.0] (family 0, port 9000)
> +
> +On client, run 3 nc commands in different terminals::
> +
> + nstatuser@nstat-a:~$ nc -v nstat-b 9000
> + Connection to nstat-b 9000 port [tcp/*] succeeded!
> +
> +The nc command only accepts 1 connection, and the accept queue length
> +is 1. On current linux implementation, set queue length to n means the
> +actual queue length is n+1. Now we create 3 connections, 1 is accepted
> +by nc, 2 in accepted queue, so the accept queue is full.
> +
> +Before running the 4th nc, we clean the nstat history on the server:
> +
> + nstatuser@nstat-b:~$ nstat -n
> +
> +Run the 4th nc on the client:
> +
> + nstatuser@nstat-a:~$ nc -v nstat-b 9000
> +
> +If the nc server is running on kernel 4.15 or higher version, you
> +won't see the "Connection to ... succeeded!" string, because kernel
> +will drop the syn if the accept queue is full. If the nc client is running
> +on an old kernel, you could see that the connection is succeeded,
> +because kernel would complete the 3-way handshake and keep the socket
> +on the half-open queue.
> +
> +Our test is on kernel 4.15, run nstat on the server:
> +
> + nstatuser@nstat-b:~$ nstat
> + #kernel
> + IpInReceives 4 0.0
> + IpInDelivers 4 0.0
> + TcpInSegs 4 0.0
> + TcpExtListenOverflows 4 0.0
> + TcpExtListenDrops 4 0.0
> + IpExtInOctets 240 0.0
> + IpExtInNoECTPkts 4 0.0
> +
> +We can see both TcpExtListenOverflows and TcpExtListenDrops are 4. If
> +the time between the 4th nc and the nstat is longer, the value of
> +TcpExtListenOverflows and TcpExtListenDrops will be larger, because
> +the syn of the 4th nc is dropped, it keeps retrying.
> +
> --
> 2.17.1
>
^ permalink raw reply
* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: Yunsheng Lin @ 2018-11-10 2:44 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpWryiOQg23fZ2iveM25Rc-V8i5uvph4ezsD2dimqF4doA@mail.gmail.com>
On 2018/11/10 10:09, Cong Wang wrote:
> On Fri, Nov 9, 2018 at 6:02 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>
>> On 2018/11/10 9:42, Cong Wang wrote:
>>> On Fri, Nov 9, 2018 at 5:39 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>>>
>>>> On 2018/11/10 3:43, Cong Wang wrote:
>>>>> Currently netdev_rx_csum_fault() only shows a device name,
>>>>> we need more information about the skb for debugging.
>>>>>
>>>>> Sample output:
>>>>>
>>>>> ens3: hw csum failure
>>>>> dev features: 0x0000000000014b89
>>>>> skb len=84 data_len=0 gso_size=0 gso_type=0 ip_summed=0 csum=0, csum_complete_sw=0, csum_valid=0
>>>>>
>>>>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>>>> ---
>>>>> include/linux/netdevice.h | 5 +++--
>>>>> net/core/datagram.c | 6 +++---
>>>>> net/core/dev.c | 10 ++++++++--
>>>>> net/sunrpc/socklib.c | 2 +-
>>>>> 4 files changed, 15 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>>> index 857f8abf7b91..fabcd9fa6cf7 100644
>>>>> --- a/include/linux/netdevice.h
>>>>> +++ b/include/linux/netdevice.h
>>>>> @@ -4332,9 +4332,10 @@ static inline bool can_checksum_protocol(netdev_features_t features,
>>>>> }
>>>>>
>>>>> #ifdef CONFIG_BUG
>>>>> -void netdev_rx_csum_fault(struct net_device *dev);
>>>>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb);
>>>>> #else
>>>>> -static inline void netdev_rx_csum_fault(struct net_device *dev)
>>>>> +static inline void netdev_rx_csum_fault(struct net_device *dev,
>>>>> + struct sk_buff *skb)
>>>>> {
>>>>> }
>>>>> #endif
>>>>> diff --git a/net/core/datagram.c b/net/core/datagram.c
>>>>> index 57f3a6fcfc1e..d8f4d55cd6c5 100644
>>>>> --- a/net/core/datagram.c
>>>>> +++ b/net/core/datagram.c
>>>>> @@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
>>>>> if (likely(!sum)) {
>>>>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>>>>> !skb->csum_complete_sw)
>>>>> - netdev_rx_csum_fault(skb->dev);
>>>>> + netdev_rx_csum_fault(skb->dev, skb);
>>>>> }
>>>>> if (!skb_shared(skb))
>>>>> skb->csum_valid = !sum;
>>>>> @@ -756,7 +756,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
>>>>> if (likely(!sum)) {
>>>>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>>>>> !skb->csum_complete_sw)
>>>>> - netdev_rx_csum_fault(skb->dev);
>>>>> + netdev_rx_csum_fault(skb->dev, skb);
>>>>> }
>>>>>
>>>>> if (!skb_shared(skb)) {
>>>>> @@ -810,7 +810,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
>>>>>
>>>>> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
>>>>> !skb->csum_complete_sw)
>>>>> - netdev_rx_csum_fault(NULL);
>>>>> + netdev_rx_csum_fault(NULL, skb);
>>>>> }
>>>>> return 0;
>>>>> fault:
>>>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>>>> index 0ffcbdd55fa9..2b337df26117 100644
>>>>> --- a/net/core/dev.c
>>>>> +++ b/net/core/dev.c
>>>>> @@ -3091,10 +3091,16 @@ EXPORT_SYMBOL(__skb_gso_segment);
>>>>>
>>>>> /* Take action when hardware reception checksum errors are detected. */
>>>>> #ifdef CONFIG_BUG
>>>>> -void netdev_rx_csum_fault(struct net_device *dev)
>>>>> +void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
>>>>> {
>>>>> if (net_ratelimit()) {
>>>>> pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
>>>>> + if (dev)
>>>>> + pr_err("dev features: %pNF\n", &dev->features);
>>>>> + pr_err("skb len=%d data_len=%d gso_size=%d gso_type=%d ip_summed=%d csum=%x, csum_complete_sw=%d, csum_valid=%d\n",
>>>>> + skb->len, skb->data_len, skb_shinfo(skb)->gso_size,
>>>>> + skb_shinfo(skb)->gso_type, skb->ip_summed, skb->csum,
>>>>> + skb->csum_complete_sw, skb->csum_valid);
>>>>
>>>>
>>>> This function also have the netdev available, use netdev_err to log the error?
>>>
>>> It is apparently not me who picked pr_err() from the beginning,
>>> I just follow that pr_err(). If you are not happy with it, please send
>>> a followup.
>>
>> Yes, but perhaps it is something to improve.
>
>
> Sure, no one stops you from improving it in a followup patch. :)
>
>
>> When using the netdev, then maybe it does not have to check if dev is null, because
>> netdev_err has handled the netdev being NULL case.
>> Maybe I missed something that netdev can not be used here?
>> If not, maybe I can send a followup.
>>
>
> Maybe. Again, my patch intends to add a few debugging logs,
> not to convert pr_err() to whatever else, they are totally different
> goals. I choose pr_err() only because I follow the existing one,
> not to say which one is better than the other.
Ok. :)
>
> Thanks.
>
> .
>
^ permalink raw reply
* [PATCH net-next] nfp: use the new __netdev_tx_sent_queue() BQL optimisation
From: Jakub Kicinski @ 2018-11-10 2:50 UTC (permalink / raw)
To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
__netdev_tx_sent_queue() was added in commit e59020abf0f
("net: bql: add __netdev_tx_sent_queue()") and allows for
better GSO performance.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
---
drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index b55c91818a67..9aa6265bf4de 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -890,8 +890,6 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
u64_stats_update_end(&r_vec->tx_sync);
}
- netdev_tx_sent_queue(nd_q, txbuf->real_len);
-
skb_tx_timestamp(skb);
tx_ring->wr_p += nr_frags + 1;
@@ -899,7 +897,7 @@ static int nfp_net_tx(struct sk_buff *skb, struct net_device *netdev)
nfp_net_tx_ring_stop(nd_q, tx_ring);
tx_ring->wr_ptr_add += nr_frags + 1;
- if (!skb->xmit_more || netif_xmit_stopped(nd_q))
+ if (__netdev_tx_sent_queue(nd_q, txbuf->real_len, skb->xmit_more))
nfp_net_tx_xmit_more_flush(tx_ring);
return NETDEV_TX_OK;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 0/8] More accurate PHC<->system clock synchronization
From: David Miller @ 2018-11-10 3:44 UTC (permalink / raw)
To: richardcochran
Cc: mlichvar, netdev, jacob.e.keller, mtosatti, jeffrey.t.kirsher,
michael.chan
In-Reply-To: <20181110014431.gc3yqjujsnjib2ue@localhost>
From: Richard Cochran <richardcochran@gmail.com>
Date: Fri, 9 Nov 2018 17:44:31 -0800
> On Fri, Nov 09, 2018 at 03:28:46PM -0800, David Miller wrote:
>> This series looks good to me but I want to give Richard an opportunity to
>> review it first.
>
> The series is good to go.
>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
Great, series applied to net-next, thanks everyone.
^ permalink raw reply
* Re: [PATCH net] net: qualcomm: rmnet: Fix incorrect assignment of real_dev
From: David Miller @ 2018-11-10 3:46 UTC (permalink / raw)
To: subashab; +Cc: netdev, stranche
In-Reply-To: <1541814987-28403-1-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Fri, 9 Nov 2018 18:56:27 -0700
> A null dereference was observed when a sysctl was being set
> from userspace and rmnet was stuck trying to complete some actions
> in the NETDEV_REGISTER callback. This is because the real_dev is set
> only after the device registration handler completes.
...
> Fixes: b752eff5be24 ("net: qualcomm: rmnet: Implement ndo_get_iflink")
> Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] nfp: use the new __netdev_tx_sent_queue() BQL optimisation
From: David Miller @ 2018-11-10 3:49 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20181110025000.25793-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 9 Nov 2018 18:50:00 -0800
> __netdev_tx_sent_queue() was added in commit e59020abf0f
> ("net: bql: add __netdev_tx_sent_queue()") and allows for
> better GSO performance.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
Applied.
^ permalink raw reply
* [PATCH net-next 2/6] nfp: flower: allow non repr netdev offload
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Previously the offload functions in NFP assumed that the ingress (or
egress) netdev passed to them was an nfp repr.
Modify the driver to permit the passing of non repr netdevs as the ingress
device for an offload rule candidate. This may include devices such as
tunnels. The driver should then base its offload decision on a combination
of ingress device and egress port for a rule.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../ethernet/netronome/nfp/flower/action.c | 14 +++----
.../net/ethernet/netronome/nfp/flower/main.h | 3 +-
.../net/ethernet/netronome/nfp/flower/match.c | 38 ++++++++++---------
.../ethernet/netronome/nfp/flower/offload.c | 33 +++++++++-------
4 files changed, 49 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index fbc052d5bb47..2e64fe878da6 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -149,11 +149,12 @@ nfp_fl_output(struct nfp_app *app, struct nfp_fl_output *output,
/* Set action output parameters. */
output->flags = cpu_to_be16(tmp_flags);
- /* Only offload if egress ports are on the same device as the
- * ingress port.
- */
- if (!switchdev_port_same_parent_id(in_dev, out_dev))
- return -EOPNOTSUPP;
+ if (nfp_netdev_is_nfp_repr(in_dev)) {
+ /* Confirm ingress and egress are on same device. */
+ if (!switchdev_port_same_parent_id(in_dev, out_dev))
+ return -EOPNOTSUPP;
+ }
+
if (!nfp_netdev_is_nfp_repr(out_dev))
return -EOPNOTSUPP;
@@ -840,9 +841,8 @@ nfp_flower_loop_action(struct nfp_app *app, const struct tc_action *a,
*a_len += sizeof(struct nfp_fl_push_vlan);
} else if (is_tcf_tunnel_set(a)) {
struct ip_tunnel_info *ip_tun = tcf_tunnel_info(a);
- struct nfp_repr *repr = netdev_priv(netdev);
- *tun_type = nfp_fl_get_tun_from_act_l4_port(repr->app, a);
+ *tun_type = nfp_fl_get_tun_from_act_l4_port(app, a);
if (*tun_type == NFP_FL_TUNNEL_NONE)
return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 0f6f1675f6f1..4a2b1a915131 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -222,7 +222,8 @@ void nfp_flower_metadata_cleanup(struct nfp_app *app);
int nfp_flower_setup_tc(struct nfp_app *app, struct net_device *netdev,
enum tc_setup_type type, void *type_data);
-int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow,
+int nfp_flower_compile_flow_match(struct nfp_app *app,
+ struct tc_cls_flower_offload *flow,
struct nfp_fl_key_ls *key_ls,
struct net_device *netdev,
struct nfp_fl_payload *nfp_flow,
diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
index e54fb6034326..cdf75595f627 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/match.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
@@ -52,10 +52,13 @@ nfp_flower_compile_port(struct nfp_flower_in_port *frame, u32 cmsg_port,
return 0;
}
- if (tun_type)
+ if (tun_type) {
frame->in_port = cpu_to_be32(NFP_FL_PORT_TYPE_TUN | tun_type);
- else
+ } else {
+ if (!cmsg_port)
+ return -EOPNOTSUPP;
frame->in_port = cpu_to_be32(cmsg_port);
+ }
return 0;
}
@@ -289,17 +292,21 @@ nfp_flower_compile_ipv4_udp_tun(struct nfp_flower_ipv4_udp_tun *frame,
}
}
-int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow,
+int nfp_flower_compile_flow_match(struct nfp_app *app,
+ struct tc_cls_flower_offload *flow,
struct nfp_fl_key_ls *key_ls,
struct net_device *netdev,
struct nfp_fl_payload *nfp_flow,
enum nfp_flower_tun_type tun_type)
{
- struct nfp_repr *netdev_repr;
+ u32 cmsg_port = 0;
int err;
u8 *ext;
u8 *msk;
+ if (nfp_netdev_is_nfp_repr(netdev))
+ cmsg_port = nfp_repr_get_port_id(netdev);
+
memset(nfp_flow->unmasked_data, 0, key_ls->key_size);
memset(nfp_flow->mask_data, 0, key_ls->key_size);
@@ -327,15 +334,13 @@ int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow,
/* Populate Exact Port data. */
err = nfp_flower_compile_port((struct nfp_flower_in_port *)ext,
- nfp_repr_get_port_id(netdev),
- false, tun_type);
+ cmsg_port, false, tun_type);
if (err)
return err;
/* Populate Mask Port Data. */
err = nfp_flower_compile_port((struct nfp_flower_in_port *)msk,
- nfp_repr_get_port_id(netdev),
- true, tun_type);
+ cmsg_port, true, tun_type);
if (err)
return err;
@@ -399,16 +404,13 @@ int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow,
msk += sizeof(struct nfp_flower_ipv4_udp_tun);
/* Configure tunnel end point MAC. */
- if (nfp_netdev_is_nfp_repr(netdev)) {
- netdev_repr = netdev_priv(netdev);
- nfp_tunnel_write_macs(netdev_repr->app);
-
- /* Store the tunnel destination in the rule data.
- * This must be present and be an exact match.
- */
- nfp_flow->nfp_tun_ipv4_addr = tun_dst;
- nfp_tunnel_add_ipv4_off(netdev_repr->app, tun_dst);
- }
+ nfp_tunnel_write_macs(app);
+
+ /* Store the tunnel destination in the rule data.
+ * This must be present and be an exact match.
+ */
+ nfp_flow->nfp_tun_ipv4_addr = tun_dst;
+ nfp_tunnel_add_ipv4_off(app, tun_dst);
if (key_ls->key_layer_two & NFP_FLOWER_LAYER2_GENEVE_OP) {
err = nfp_flower_compile_geneve_opt(ext, flow, false);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 29c95423ab64..2c32edfc1a9d 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -56,11 +56,10 @@
BIT(FLOW_DISSECTOR_KEY_ENC_PORTS))
static int
-nfp_flower_xmit_flow(struct net_device *netdev,
- struct nfp_fl_payload *nfp_flow, u8 mtype)
+nfp_flower_xmit_flow(struct nfp_app *app, struct nfp_fl_payload *nfp_flow,
+ u8 mtype)
{
u32 meta_len, key_len, mask_len, act_len, tot_len;
- struct nfp_repr *priv = netdev_priv(netdev);
struct sk_buff *skb;
unsigned char *msg;
@@ -78,7 +77,7 @@ nfp_flower_xmit_flow(struct net_device *netdev,
nfp_flow->meta.mask_len >>= NFP_FL_LW_SIZ;
nfp_flow->meta.act_len >>= NFP_FL_LW_SIZ;
- skb = nfp_flower_cmsg_alloc(priv->app, tot_len, mtype, GFP_KERNEL);
+ skb = nfp_flower_cmsg_alloc(app, tot_len, mtype, GFP_KERNEL);
if (!skb)
return -ENOMEM;
@@ -96,7 +95,7 @@ nfp_flower_xmit_flow(struct net_device *netdev,
nfp_flow->meta.mask_len <<= NFP_FL_LW_SIZ;
nfp_flow->meta.act_len <<= NFP_FL_LW_SIZ;
- nfp_ctrl_tx(priv->app->ctrl, skb);
+ nfp_ctrl_tx(app->ctrl, skb);
return 0;
}
@@ -427,13 +426,16 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
struct tc_cls_flower_offload *flow, bool egress)
{
enum nfp_flower_tun_type tun_type = NFP_FL_TUNNEL_NONE;
- struct nfp_port *port = nfp_port_from_netdev(netdev);
struct nfp_flower_priv *priv = app->priv;
struct nfp_fl_payload *flow_pay;
struct nfp_fl_key_ls *key_layer;
+ struct nfp_port *port = NULL;
struct net_device *ingr_dev;
int err;
+ if (nfp_netdev_is_nfp_repr(netdev))
+ port = nfp_port_from_netdev(netdev);
+
ingr_dev = egress ? NULL : netdev;
flow_pay = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
NFP_FL_STATS_CTX_DONT_CARE);
@@ -462,8 +464,8 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
flow_pay->ingress_dev = egress ? NULL : netdev;
- err = nfp_flower_compile_flow_match(flow, key_layer, netdev, flow_pay,
- tun_type);
+ err = nfp_flower_compile_flow_match(app, flow, key_layer, netdev,
+ flow_pay, tun_type);
if (err)
goto err_destroy_flow;
@@ -476,7 +478,7 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
if (err)
goto err_destroy_flow;
- err = nfp_flower_xmit_flow(netdev, flow_pay,
+ err = nfp_flower_xmit_flow(app, flow_pay,
NFP_FLOWER_CMSG_TYPE_FLOW_ADD);
if (err)
goto err_destroy_flow;
@@ -487,7 +489,8 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
if (err)
goto err_destroy_flow;
- port->tc_offload_cnt++;
+ if (port)
+ port->tc_offload_cnt++;
/* Deallocate flow payload when flower rule has been destroyed. */
kfree(key_layer);
@@ -520,12 +523,15 @@ static int
nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
struct tc_cls_flower_offload *flow, bool egress)
{
- struct nfp_port *port = nfp_port_from_netdev(netdev);
struct nfp_flower_priv *priv = app->priv;
struct nfp_fl_payload *nfp_flow;
+ struct nfp_port *port = NULL;
struct net_device *ingr_dev;
int err;
+ if (nfp_netdev_is_nfp_repr(netdev))
+ port = nfp_port_from_netdev(netdev);
+
ingr_dev = egress ? NULL : netdev;
nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
NFP_FL_STATS_CTX_DONT_CARE);
@@ -539,13 +545,14 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
if (nfp_flow->nfp_tun_ipv4_addr)
nfp_tunnel_del_ipv4_off(app, nfp_flow->nfp_tun_ipv4_addr);
- err = nfp_flower_xmit_flow(netdev, nfp_flow,
+ err = nfp_flower_xmit_flow(app, nfp_flow,
NFP_FLOWER_CMSG_TYPE_FLOW_DEL);
if (err)
goto err_free_flow;
err_free_flow:
- port->tc_offload_cnt--;
+ if (port)
+ port->tc_offload_cnt--;
kfree(nfp_flow->action_data);
kfree(nfp_flow->mask_data);
kfree(nfp_flow->unmasked_data);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/6] nfp: flower: increase scope of netdev checking functions
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Both the actions and tunnel_conf files contain local functions that check
the type of an input netdev. In preparation for re-use with tunnel offload
via indirect blocks, move these to static inline functions in a header
file.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../ethernet/netronome/nfp/flower/action.c | 14 ----------
.../net/ethernet/netronome/nfp/flower/cmsg.h | 27 +++++++++++++++++++
.../netronome/nfp/flower/tunnel_conf.c | 19 ++-----------
3 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/action.c b/drivers/net/ethernet/netronome/nfp/flower/action.c
index 2e64fe878da6..8d54b36afee8 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/action.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/action.c
@@ -2,7 +2,6 @@
/* Copyright (C) 2017-2018 Netronome Systems, Inc. */
#include <linux/bitfield.h>
-#include <net/geneve.h>
#include <net/pkt_cls.h>
#include <net/switchdev.h>
#include <net/tc_act/tc_csum.h>
@@ -11,7 +10,6 @@
#include <net/tc_act/tc_pedit.h>
#include <net/tc_act/tc_vlan.h>
#include <net/tc_act/tc_tunnel_key.h>
-#include <net/vxlan.h>
#include "cmsg.h"
#include "main.h"
@@ -92,18 +90,6 @@ nfp_fl_pre_lag(struct nfp_app *app, const struct tc_action *action,
return act_size;
}
-static bool nfp_fl_netdev_is_tunnel_type(struct net_device *out_dev,
- enum nfp_flower_tun_type tun_type)
-{
- if (netif_is_vxlan(out_dev))
- return tun_type == NFP_FL_TUNNEL_VXLAN;
-
- if (netif_is_geneve(out_dev))
- return tun_type == NFP_FL_TUNNEL_GENEVE;
-
- return false;
-}
-
static int
nfp_fl_output(struct nfp_app *app, struct nfp_fl_output *output,
const struct tc_action *action, struct nfp_fl_payload *nfp_flow,
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
index 3e391555e191..15f41cfef9f1 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h
@@ -8,6 +8,7 @@
#include <linux/skbuff.h>
#include <linux/types.h>
#include <net/geneve.h>
+#include <net/vxlan.h>
#include "../nfp_app.h"
#include "../nfpcore/nfp_cpp.h"
@@ -499,6 +500,32 @@ static inline int nfp_flower_cmsg_get_data_len(struct sk_buff *skb)
return skb->len - NFP_FLOWER_CMSG_HLEN;
}
+static inline bool
+nfp_fl_netdev_is_tunnel_type(struct net_device *netdev,
+ enum nfp_flower_tun_type tun_type)
+{
+ if (netif_is_vxlan(netdev))
+ return tun_type == NFP_FL_TUNNEL_VXLAN;
+ if (netif_is_geneve(netdev))
+ return tun_type == NFP_FL_TUNNEL_GENEVE;
+
+ return false;
+}
+
+static inline bool nfp_fl_is_netdev_to_offload(struct net_device *netdev)
+{
+ if (!netdev->rtnl_link_ops)
+ return false;
+ if (!strcmp(netdev->rtnl_link_ops->kind, "openvswitch"))
+ return true;
+ if (netif_is_vxlan(netdev))
+ return true;
+ if (netif_is_geneve(netdev))
+ return true;
+
+ return false;
+}
+
struct sk_buff *
nfp_flower_cmsg_mac_repr_start(struct nfp_app *app, unsigned int num_ports);
void
diff --git a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
index 5d641d7dabff..2d9f26a725c2 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
@@ -4,7 +4,6 @@
#include <linux/etherdevice.h>
#include <linux/inetdevice.h>
#include <net/netevent.h>
-#include <net/vxlan.h>
#include <linux/idr.h>
#include <net/dst_metadata.h>
#include <net/arp.h>
@@ -182,20 +181,6 @@ void nfp_tunnel_keep_alive(struct nfp_app *app, struct sk_buff *skb)
}
}
-static bool nfp_tun_is_netdev_to_offload(struct net_device *netdev)
-{
- if (!netdev->rtnl_link_ops)
- return false;
- if (!strcmp(netdev->rtnl_link_ops->kind, "openvswitch"))
- return true;
- if (netif_is_vxlan(netdev))
- return true;
- if (netif_is_geneve(netdev))
- return true;
-
- return false;
-}
-
static int
nfp_flower_xmit_tun_conf(struct nfp_app *app, u8 mtype, u16 plen, void *pdata,
gfp_t flag)
@@ -617,7 +602,7 @@ static void nfp_tun_add_to_mac_offload_list(struct net_device *netdev,
if (nfp_netdev_is_nfp_repr(netdev))
port = nfp_repr_get_port_id(netdev);
- else if (!nfp_tun_is_netdev_to_offload(netdev))
+ else if (!nfp_fl_is_netdev_to_offload(netdev))
return;
entry = kmalloc(sizeof(*entry), GFP_KERNEL);
@@ -660,7 +645,7 @@ int nfp_tunnel_mac_event_handler(struct nfp_app *app,
{
if (event == NETDEV_DOWN || event == NETDEV_UNREGISTER) {
/* If non-nfp netdev then free its offload index. */
- if (nfp_tun_is_netdev_to_offload(netdev))
+ if (nfp_fl_is_netdev_to_offload(netdev))
nfp_tun_del_mac_idx(app, netdev->ifindex);
} else if (event == NETDEV_UP || event == NETDEV_CHANGEADDR ||
event == NETDEV_REGISTER) {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/6] net: sched: register callbacks for indirect tc block binds
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley, Jakub Kicinski
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Currently drivers can register to receive TC block bind/unbind callbacks
by implementing the setup_tc ndo in any of their given netdevs. However,
drivers may also be interested in binds to higher level devices (e.g.
tunnel drivers) to potentially offload filters applied to them.
Introduce indirect block devs which allows drivers to register callbacks
for block binds on other devices. The callback is triggered when the
device is bound to a block, allowing the driver to register for rules
applied to that block using already available functions.
Freeing an indirect block callback will trigger an unbind event (if
necessary) to direct the driver to remove any offloaded rules and unreg
any block rule callbacks. It is the responsibility of the implementing
driver to clean any registered indirect block callbacks before exiting,
if the block it still active at such a time.
Allow registering an indirect block dev callback for a device that is
already bound to a block. In this case (if it is an ingress block),
register and also trigger the callback meaning that any already installed
rules can be replayed to the calling driver.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/net/pkt_cls.h | 34 +++++
include/net/sch_generic.h | 3 +
net/sched/cls_api.c | 256 +++++++++++++++++++++++++++++++++++++-
3 files changed, 292 insertions(+), 1 deletion(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 00f71644fbcd..f6c0cd29dea4 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -81,6 +81,14 @@ void __tcf_block_cb_unregister(struct tcf_block *block,
struct tcf_block_cb *block_cb);
void tcf_block_cb_unregister(struct tcf_block *block,
tc_setup_cb_t *cb, void *cb_ident);
+int __tc_indr_block_cb_register(struct net_device *dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident);
+int tc_indr_block_cb_register(struct net_device *dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident);
+void __tc_indr_block_cb_unregister(struct net_device *dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident);
+void tc_indr_block_cb_unregister(struct net_device *dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident);
int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res, bool compat_mode);
@@ -183,6 +191,32 @@ void tcf_block_cb_unregister(struct tcf_block *block,
{
}
+static inline
+int __tc_indr_block_cb_register(struct net_device *dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ return 0;
+}
+
+static inline
+int tc_indr_block_cb_register(struct net_device *dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ return 0;
+}
+
+static inline
+void __tc_indr_block_cb_unregister(struct net_device *dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+}
+
+static inline
+void tc_indr_block_cb_unregister(struct net_device *dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+}
+
static inline int tcf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res, bool compat_mode)
{
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index a8dd1fc141b6..9481f2c142e2 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -24,6 +24,9 @@ struct bpf_flow_keys;
typedef int tc_setup_cb_t(enum tc_setup_type type,
void *type_data, void *cb_priv);
+typedef int tc_indr_block_bind_cb_t(struct net_device *dev, void *cb_priv,
+ enum tc_setup_type type, void *type_data);
+
struct qdisc_rate_table {
struct tc_ratespec rate;
u32 data[256];
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f427a1e00e7e..d92f44ac4c39 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -25,6 +25,7 @@
#include <linux/kmod.h>
#include <linux/slab.h>
#include <linux/idr.h>
+#include <linux/rhashtable.h>
#include <net/net_namespace.h>
#include <net/sock.h>
#include <net/netlink.h>
@@ -365,6 +366,245 @@ static void tcf_chain_flush(struct tcf_chain *chain)
}
}
+static struct tcf_block *tc_dev_ingress_block(struct net_device *dev)
+{
+ const struct Qdisc_class_ops *cops;
+ struct Qdisc *qdisc;
+
+ if (!dev_ingress_queue(dev))
+ return NULL;
+
+ qdisc = dev_ingress_queue(dev)->qdisc_sleeping;
+ if (!qdisc)
+ return NULL;
+
+ cops = qdisc->ops->cl_ops;
+ if (!cops)
+ return NULL;
+
+ if (!cops->tcf_block)
+ return NULL;
+
+ return cops->tcf_block(qdisc, TC_H_MIN_INGRESS, NULL);
+}
+
+static struct rhashtable indr_setup_block_ht;
+
+struct tc_indr_block_dev {
+ struct rhash_head ht_node;
+ struct net_device *dev;
+ unsigned int refcnt;
+ struct list_head cb_list;
+ struct tcf_block *block;
+};
+
+struct tc_indr_block_cb {
+ struct list_head list;
+ void *cb_priv;
+ tc_indr_block_bind_cb_t *cb;
+ void *cb_ident;
+};
+
+static const struct rhashtable_params tc_indr_setup_block_ht_params = {
+ .key_offset = offsetof(struct tc_indr_block_dev, dev),
+ .head_offset = offsetof(struct tc_indr_block_dev, ht_node),
+ .key_len = sizeof(struct net_device *),
+};
+
+static struct tc_indr_block_dev *
+tc_indr_block_dev_lookup(struct net_device *dev)
+{
+ return rhashtable_lookup_fast(&indr_setup_block_ht, &dev,
+ tc_indr_setup_block_ht_params);
+}
+
+static struct tc_indr_block_dev *tc_indr_block_dev_get(struct net_device *dev)
+{
+ struct tc_indr_block_dev *indr_dev;
+
+ indr_dev = tc_indr_block_dev_lookup(dev);
+ if (indr_dev)
+ goto inc_ref;
+
+ indr_dev = kzalloc(sizeof(*indr_dev), GFP_KERNEL);
+ if (!indr_dev)
+ return NULL;
+
+ INIT_LIST_HEAD(&indr_dev->cb_list);
+ indr_dev->dev = dev;
+ indr_dev->block = tc_dev_ingress_block(dev);
+ if (rhashtable_insert_fast(&indr_setup_block_ht, &indr_dev->ht_node,
+ tc_indr_setup_block_ht_params)) {
+ kfree(indr_dev);
+ return NULL;
+ }
+
+inc_ref:
+ indr_dev->refcnt++;
+ return indr_dev;
+}
+
+static void tc_indr_block_dev_put(struct tc_indr_block_dev *indr_dev)
+{
+ if (--indr_dev->refcnt)
+ return;
+
+ rhashtable_remove_fast(&indr_setup_block_ht, &indr_dev->ht_node,
+ tc_indr_setup_block_ht_params);
+ kfree(indr_dev);
+}
+
+static struct tc_indr_block_cb *
+tc_indr_block_cb_lookup(struct tc_indr_block_dev *indr_dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ struct tc_indr_block_cb *indr_block_cb;
+
+ list_for_each_entry(indr_block_cb, &indr_dev->cb_list, list)
+ if (indr_block_cb->cb == cb &&
+ indr_block_cb->cb_ident == cb_ident)
+ return indr_block_cb;
+ return NULL;
+}
+
+static struct tc_indr_block_cb *
+tc_indr_block_cb_add(struct tc_indr_block_dev *indr_dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ struct tc_indr_block_cb *indr_block_cb;
+
+ indr_block_cb = tc_indr_block_cb_lookup(indr_dev, cb, cb_ident);
+ if (indr_block_cb)
+ return ERR_PTR(-EEXIST);
+
+ indr_block_cb = kzalloc(sizeof(*indr_block_cb), GFP_KERNEL);
+ if (!indr_block_cb)
+ return ERR_PTR(-ENOMEM);
+
+ indr_block_cb->cb_priv = cb_priv;
+ indr_block_cb->cb = cb;
+ indr_block_cb->cb_ident = cb_ident;
+ list_add(&indr_block_cb->list, &indr_dev->cb_list);
+
+ return indr_block_cb;
+}
+
+static void tc_indr_block_cb_del(struct tc_indr_block_cb *indr_block_cb)
+{
+ list_del(&indr_block_cb->list);
+ kfree(indr_block_cb);
+}
+
+static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
+ struct tc_indr_block_cb *indr_block_cb,
+ enum tc_block_command command)
+{
+ struct tc_block_offload bo = {
+ .command = command,
+ .binder_type = TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS,
+ .block = indr_dev->block,
+ };
+
+ if (!indr_dev->block)
+ return;
+
+ indr_block_cb->cb(indr_dev->dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
+ &bo);
+}
+
+int __tc_indr_block_cb_register(struct net_device *dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ struct tc_indr_block_cb *indr_block_cb;
+ struct tc_indr_block_dev *indr_dev;
+ int err;
+
+ indr_dev = tc_indr_block_dev_get(dev);
+ if (!indr_dev)
+ return -ENOMEM;
+
+ indr_block_cb = tc_indr_block_cb_add(indr_dev, cb_priv, cb, cb_ident);
+ err = PTR_ERR_OR_ZERO(indr_block_cb);
+ if (err)
+ goto err_dev_put;
+
+ tc_indr_block_ing_cmd(indr_dev, indr_block_cb, TC_BLOCK_BIND);
+ return 0;
+
+err_dev_put:
+ tc_indr_block_dev_put(indr_dev);
+ return err;
+}
+EXPORT_SYMBOL_GPL(__tc_indr_block_cb_register);
+
+int tc_indr_block_cb_register(struct net_device *dev, void *cb_priv,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ int err;
+
+ rtnl_lock();
+ err = __tc_indr_block_cb_register(dev, cb_priv, cb, cb_ident);
+ rtnl_unlock();
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(tc_indr_block_cb_register);
+
+void __tc_indr_block_cb_unregister(struct net_device *dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ struct tc_indr_block_cb *indr_block_cb;
+ struct tc_indr_block_dev *indr_dev;
+
+ indr_dev = tc_indr_block_dev_lookup(dev);
+ if (!indr_dev)
+ return;
+
+ indr_block_cb = tc_indr_block_cb_lookup(indr_dev, cb, cb_ident);
+ if (!indr_block_cb)
+ return;
+
+ /* Send unbind message if required to free any block cbs. */
+ tc_indr_block_ing_cmd(indr_dev, indr_block_cb, TC_BLOCK_UNBIND);
+ tc_indr_block_cb_del(indr_block_cb);
+ tc_indr_block_dev_put(indr_dev);
+}
+EXPORT_SYMBOL_GPL(__tc_indr_block_cb_unregister);
+
+void tc_indr_block_cb_unregister(struct net_device *dev,
+ tc_indr_block_bind_cb_t *cb, void *cb_ident)
+{
+ rtnl_lock();
+ __tc_indr_block_cb_unregister(dev, cb, cb_ident);
+ rtnl_unlock();
+}
+EXPORT_SYMBOL_GPL(tc_indr_block_cb_unregister);
+
+static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
+ struct tcf_block_ext_info *ei,
+ enum tc_block_command command,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_indr_block_cb *indr_block_cb;
+ struct tc_indr_block_dev *indr_dev;
+ struct tc_block_offload bo = {
+ .command = command,
+ .binder_type = ei->binder_type,
+ .block = block,
+ .extack = extack,
+ };
+
+ indr_dev = tc_indr_block_dev_lookup(dev);
+ if (!indr_dev)
+ return;
+
+ indr_dev->block = command == TC_BLOCK_BIND ? block : NULL;
+
+ list_for_each_entry(indr_block_cb, &indr_dev->cb_list, list)
+ indr_block_cb->cb(dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
+ &bo);
+}
+
static bool tcf_block_offload_in_use(struct tcf_block *block)
{
return block->offloadcnt;
@@ -406,12 +646,17 @@ static int tcf_block_offload_bind(struct tcf_block *block, struct Qdisc *q,
err = tcf_block_offload_cmd(block, dev, ei, TC_BLOCK_BIND, extack);
if (err == -EOPNOTSUPP)
goto no_offload_dev_inc;
- return err;
+ if (err)
+ return err;
+
+ tc_indr_block_call(block, dev, ei, TC_BLOCK_BIND, extack);
+ return 0;
no_offload_dev_inc:
if (tcf_block_offload_in_use(block))
return -EOPNOTSUPP;
block->nooffloaddevcnt++;
+ tc_indr_block_call(block, dev, ei, TC_BLOCK_BIND, extack);
return 0;
}
@@ -421,6 +666,8 @@ static void tcf_block_offload_unbind(struct tcf_block *block, struct Qdisc *q,
struct net_device *dev = q->dev_queue->dev;
int err;
+ tc_indr_block_call(block, dev, ei, TC_BLOCK_UNBIND, NULL);
+
if (!dev->netdev_ops->ndo_setup_tc)
goto no_offload_dev_dec;
err = tcf_block_offload_cmd(block, dev, ei, TC_BLOCK_UNBIND, NULL);
@@ -2355,6 +2602,11 @@ static int __init tc_filter_init(void)
if (err)
goto err_register_pernet_subsys;
+ err = rhashtable_init(&indr_setup_block_ht,
+ &tc_indr_setup_block_ht_params);
+ if (err)
+ goto err_rhash_setup_block_ht;
+
rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_new_tfilter, NULL, 0);
rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_del_tfilter, NULL, 0);
rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_get_tfilter,
@@ -2366,6 +2618,8 @@ static int __init tc_filter_init(void)
return 0;
+err_rhash_setup_block_ht:
+ unregister_pernet_subsys(&tcf_net_ops);
err_register_pernet_subsys:
destroy_workqueue(tc_filter_wq);
return err;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: phy: improve struct phy_device member interrupts handling
From: David Miller @ 2018-11-10 4:12 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <63a44097-c37d-eb0f-98c8-2c1fefd1f246@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 9 Nov 2018 18:35:52 +0100
> As a heritage from the very early days of phylib member interrupts is
> defined as u32 even though it's just a flag whether interrupts are
> enabled. So we can change it to a bitfield member. In addition change
> the code dealing with this member in a way that it's clear we're
> dealing with a bool value.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - use false/true instead of 0/1 for the constants
Applied.
^ permalink raw reply
* [PATCH net v2] net: sched: cls_flower: validate nested enc_opts_policy to avoid warning
From: Jakub Kicinski @ 2018-11-10 5:06 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, Jakub Kicinski
TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
currently contain further nested attributes, which are parsed by
hand, so the policy is never actually used resulting in a W=1
build warning:
net/sched/cls_flower.c:492:1: warning: ‘enc_opts_policy’ defined but not used [-Wunused-const-variable=]
enc_opts_policy[TCA_FLOWER_KEY_ENC_OPTS_MAX + 1] = {
Add the validation anyway to avoid potential bugs when other
attributes are added and to make the attribute structure slightly
more clear. Validation will also set extact to point to bad
attribute on error.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Simon Horman <simon.horman@netronome.com>
---
net/sched/cls_flower.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9aada2d0ef06..c6c327874abc 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -709,11 +709,23 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key,
struct netlink_ext_ack *extack)
{
const struct nlattr *nla_enc_key, *nla_opt_key, *nla_opt_msk = NULL;
- int option_len, key_depth, msk_depth = 0;
+ int err, option_len, key_depth, msk_depth = 0;
+
+ err = nla_validate_nested(tb[TCA_FLOWER_KEY_ENC_OPTS],
+ TCA_FLOWER_KEY_ENC_OPTS_MAX,
+ enc_opts_policy, extack);
+ if (err)
+ return err;
nla_enc_key = nla_data(tb[TCA_FLOWER_KEY_ENC_OPTS]);
if (tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
+ err = nla_validate_nested(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK],
+ TCA_FLOWER_KEY_ENC_OPTS_MAX,
+ enc_opts_policy, extack);
+ if (err)
+ return err;
+
nla_opt_msk = nla_data(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]);
msk_depth = nla_len(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]);
}
--
2.17.1
^ permalink raw reply related
* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: David Miller @ 2018-11-10 4:16 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev
In-Reply-To: <20181109194333.19949-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 9 Nov 2018 11:43:33 -0800
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 57f3a6fcfc1e..d8f4d55cd6c5 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -736,7 +736,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
> if (likely(!sum)) {
> if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
> !skb->csum_complete_sw)
> - netdev_rx_csum_fault(skb->dev);
> + netdev_rx_csum_fault(skb->dev, skb);
> }
> if (!skb_shared(skb))
> skb->csum_valid = !sum;
Didn't you move this function into net/core/skbuff.c? :-)
Please respin.
^ permalink raw reply
* [PATCH] infiniband: nes: Fix more direct skb list accesses.
From: David Miller @ 2018-11-10 5:19 UTC (permalink / raw)
To: netdev
The following:
skb = skb->next;
...
if (skb == (struct sk_buff *)queue)
is transformed into:
skb = skb_peek_next(skb, queue);
...
if (!skb)
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/infiniband/hw/nes/nes_mgt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/nes/nes_mgt.c b/drivers/infiniband/hw/nes/nes_mgt.c
index e96ffff61c3a..fc0c191014e9 100644
--- a/drivers/infiniband/hw/nes/nes_mgt.c
+++ b/drivers/infiniband/hw/nes/nes_mgt.c
@@ -223,11 +223,11 @@ static struct sk_buff *nes_get_next_skb(struct nes_device *nesdev, struct nes_qp
}
old_skb = skb;
- skb = skb->next;
+ skb = skb_peek_next(skb, &nesqp->pau_list);
skb_unlink(old_skb, &nesqp->pau_list);
nes_mgt_free_skb(nesdev, old_skb, PCI_DMA_TODEVICE);
nes_rem_ref_cm_node(nesqp->cm_node);
- if (skb == (struct sk_buff *)&nesqp->pau_list)
+ if (!skb)
goto out;
}
return skb;
--
2.19.1
^ permalink raw reply related
* Re: [iproute2 PATCH v2] tc: flower: Classify packets based port ranges
From: Nambiar, Amritha @ 2018-11-10 5:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: stephen, netdev, jakub.kicinski, sridhar.samudrala, jhs,
xiyou.wangcong
In-Reply-To: <20181109085143.GA2050@nanopsycho>
On 11/9/2018 12:51 AM, Jiri Pirko wrote:
> Wed, Nov 07, 2018 at 10:22:50PM CET, amritha.nambiar@intel.com wrote:
>> Added support for filtering based on port ranges.
>>
>> Example:
>> 1. Match on a port range:
>> -------------------------
>> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>> prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
>> action drop
>>
>> $ tc -s filter show dev enp4s0 parent ffff:
>> filter protocol ip pref 1 flower chain 0
>> filter protocol ip pref 1 flower chain 0 handle 0x1
>> eth_type ipv4
>> ip_proto tcp
>> dst_port range 20-30
>> skip_hw
>> not_in_hw
>> action order 1: gact action drop
>> random type none pass val 0
>> index 1 ref 1 bind 1 installed 85 sec used 3 sec
>> Action statistics:
>> Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
>> backlog 0b 0p requeues 0
>>
>> 2. Match on IP address and port range:
>> --------------------------------------
>> $ tc filter add dev enp4s0 protocol ip parent ffff:\
>> prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
>> skip_hw action drop
>>
>> $ tc -s filter show dev enp4s0 parent ffff:
>> filter protocol ip pref 1 flower chain 0 handle 0x2
>> eth_type ipv4
>> ip_proto tcp
>> dst_ip 192.168.1.1
>> dst_port range 100-200
>> skip_hw
>> not_in_hw
>> action order 1: gact action drop
>> random type none pass val 0
>> index 2 ref 1 bind 1 installed 58 sec used 2 sec
>> Action statistics:
>> Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
>> backlog 0b 0p requeues 0
>>
>> v2:
>> Addressed Jiri's comment to sync output format with input
>>
>> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
>> ---
>> include/uapi/linux/pkt_cls.h | 7 ++
>> tc/f_flower.c | 145 +++++++++++++++++++++++++++++++++++++++---
>> 2 files changed, 142 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> index 401d0c1..b63c3cf 100644
>> --- a/include/uapi/linux/pkt_cls.h
>> +++ b/include/uapi/linux/pkt_cls.h
>> @@ -405,6 +405,11 @@ enum {
>> TCA_FLOWER_KEY_UDP_SRC, /* be16 */
>> TCA_FLOWER_KEY_UDP_DST, /* be16 */
>>
>> + TCA_FLOWER_KEY_PORT_SRC_MIN, /* be16 */
>> + TCA_FLOWER_KEY_PORT_SRC_MAX, /* be16 */
>> + TCA_FLOWER_KEY_PORT_DST_MIN, /* be16 */
>> + TCA_FLOWER_KEY_PORT_DST_MAX, /* be16 */
>> +
>> TCA_FLOWER_FLAGS,
>> TCA_FLOWER_KEY_VLAN_ID, /* be16 */
>> TCA_FLOWER_KEY_VLAN_PRIO, /* u8 */
>> @@ -518,6 +523,8 @@ enum {
>> TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
>> };
>>
>> +#define TCA_FLOWER_MASK_FLAGS_RANGE (1 << 0) /* Range-based match */
>> +
>> /* Match-all classifier */
>>
>> enum {
>> diff --git a/tc/f_flower.c b/tc/f_flower.c
>> index 65fca04..7724a1d 100644
>> --- a/tc/f_flower.c
>> +++ b/tc/f_flower.c
>> @@ -494,6 +494,66 @@ static int flower_parse_port(char *str, __u8 ip_proto,
>> return 0;
>> }
>>
>> +static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint type,
>> + __be16 *min_port_type,
>> + __be16 *max_port_type)
>> +{
>> + if (ip_proto == IPPROTO_TCP || ip_proto == IPPROTO_UDP ||
>> + ip_proto == IPPROTO_SCTP) {
>> + if (type == FLOWER_ENDPOINT_SRC) {
>> + *min_port_type = TCA_FLOWER_KEY_PORT_SRC_MIN;
>> + *max_port_type = TCA_FLOWER_KEY_PORT_SRC_MAX;
>> + } else {
>> + *min_port_type = TCA_FLOWER_KEY_PORT_DST_MIN;
>> + *max_port_type = TCA_FLOWER_KEY_PORT_DST_MAX;
>> + }
>> + } else {
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int flower_parse_port_range(__be16 *min, __be16 *max, __u8 ip_proto,
>> + enum flower_endpoint endpoint,
>> + struct nlmsghdr *n)
>> +{
>> + __be16 min_port_type, max_port_type;
>> +
>> + flower_port_range_attr_type(ip_proto, endpoint, &min_port_type,
>> + &max_port_type);
>> + addattr16(n, MAX_MSG, min_port_type, *min);
>> + addattr16(n, MAX_MSG, max_port_type, *max);
>> +
>> + return 0;
>> +}
>> +
>> +static int get_range(__be16 *min, __be16 *max, char *argv)
>> +{
>> + char *r;
>> +
>> + r = strchr(argv, '-');
>> + if (r) {
>> + *r = '\0';
>> + if (get_be16(min, argv, 10)) {
>> + fprintf(stderr, "invalid min range\n");
>> + return -1;
>> + }
>> + if (get_be16(max, r + 1, 10)) {
>> + fprintf(stderr, "invalid max range\n");
>> + return -1;
>> + }
>> + if (htons(*max) <= htons(*min)) {
>> + fprintf(stderr, "max value should be greater than min value\n");
>> + return -1;
>> + }
>> + } else {
>> + fprintf(stderr, "Illegal range format\n");
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> #define TCP_FLAGS_MAX_MASK 0xfff
>>
>> static int flower_parse_tcp_flags(char *str, int flags_type, int mask_type,
>> @@ -1061,20 +1121,54 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
>> return -1;
>> }
>> } else if (matches(*argv, "dst_port") == 0) {
>> + __be16 min, max;
>> +
>> NEXT_ARG();
>> - ret = flower_parse_port(*argv, ip_proto,
>> - FLOWER_ENDPOINT_DST, n);
>> - if (ret < 0) {
>> - fprintf(stderr, "Illegal \"dst_port\"\n");
>> - return -1;
>> + if (matches(*argv, "range") == 0) {
>> + NEXT_ARG();
>> + ret = get_range(&min, &max, *argv);
>> + if (ret < 0)
>> + return -1;
>> + ret = flower_parse_port_range(&min, &max,
>> + ip_proto,
>> + FLOWER_ENDPOINT_DST,
>> + n);
>> + if (ret < 0) {
>> + fprintf(stderr, "Illegal \"dst_port range\"\n");
>> + return -1;
>> + }
>> + } else {
>> + ret = flower_parse_port(*argv, ip_proto,
>> + FLOWER_ENDPOINT_DST, n);
>> + if (ret < 0) {
>> + fprintf(stderr, "Illegal \"dst_port\"\n");
>> + return -1;
>> + }
>> }
>> } else if (matches(*argv, "src_port") == 0) {
>> + __be16 min, max;
>> +
>> NEXT_ARG();
>> - ret = flower_parse_port(*argv, ip_proto,
>> - FLOWER_ENDPOINT_SRC, n);
>> - if (ret < 0) {
>> - fprintf(stderr, "Illegal \"src_port\"\n");
>> - return -1;
>> + if (matches(*argv, "range") == 0) {
>> + NEXT_ARG();
>> + ret = get_range(&min, &max, *argv);
>> + if (ret < 0)
>> + return -1;
>> + ret = flower_parse_port_range(&min, &max,
>> + ip_proto,
>> + FLOWER_ENDPOINT_SRC,
>> + n);
>> + if (ret < 0) {
>> + fprintf(stderr, "Illegal \"src_port range\"\n");
>> + return -1;
>> + }
>> + } else {
>> + ret = flower_parse_port(*argv, ip_proto,
>> + FLOWER_ENDPOINT_SRC, n);
>> + if (ret < 0) {
>> + fprintf(stderr, "Illegal \"src_port\"\n");
>> + return -1;
>> + }
>> }
>> } else if (matches(*argv, "tcp_flags") == 0) {
>> NEXT_ARG();
>> @@ -1490,6 +1584,22 @@ static void flower_print_port(char *name, struct rtattr *attr)
>> print_hu(PRINT_ANY, name, namefrm, rta_getattr_be16(attr));
>> }
>>
>> +static void flower_print_port_range(char *name, struct rtattr *min_attr,
>> + struct rtattr *max_attr)
>> +{
>> + SPRINT_BUF(namefrm);
>> + SPRINT_BUF(out);
>> + size_t done;
>> +
>> + if (!min_attr || !max_attr)
>> + return;
>> +
>> + done = sprintf(out, "%u", rta_getattr_be16(min_attr));
>> + sprintf(out + done, "-%u", rta_getattr_be16(max_attr));
>> + sprintf(namefrm, "\n %s %%s", name);
>> + print_string(PRINT_ANY, name, namefrm, out);
>> +}
>> +
>> static void flower_print_tcp_flags(const char *name, struct rtattr *flags_attr,
>> struct rtattr *mask_attr)
>> {
>> @@ -1678,6 +1788,7 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
>> struct rtattr *opt, __u32 handle)
>> {
>> struct rtattr *tb[TCA_FLOWER_MAX + 1];
>> + __be16 min_port_type, max_port_type;
>> int nl_type, nl_mask_type;
>> __be16 eth_type = 0;
>> __u8 ip_proto = 0xff;
>> @@ -1796,6 +1907,20 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
>> if (nl_type >= 0)
>> flower_print_port("src_port", tb[nl_type]);
>>
>> + if (flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_DST,
>> + &min_port_type, &max_port_type)
>> + == 0) {
>
> This is odd. Please convert to !fower_port....
Will fix in v3 and resubmit after the UAPI changes are accepted into
net-next.
>
>
>
>> + flower_print_port_range("dst_port range",
>> + tb[min_port_type], tb[max_port_type]);
>> + }
>> +
>> + if (flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_SRC,
>> + &min_port_type, &max_port_type)
>> + == 0) {
>
> Same here.
Will fix in v3.
>
>
>> + flower_print_port_range("src_port range",
>> + tb[min_port_type], tb[max_port_type]);
>> + }
>> +
>> flower_print_tcp_flags("tcp_flags", tb[TCA_FLOWER_KEY_TCP_FLAGS],
>> tb[TCA_FLOWER_KEY_TCP_FLAGS_MASK]);
>>
>>
>
> Otherwise this looks fine.
>
^ permalink raw reply
* [net-next PATCH v3] net: sched: cls_flower: Classify packets using port ranges
From: Amritha Nambiar @ 2018-11-10 0:11 UTC (permalink / raw)
To: netdev, davem
Cc: jakub.kicinski, amritha.nambiar, sridhar.samudrala, jhs,
xiyou.wangcong, jiri
Added support in tc flower for filtering based on port ranges.
Example:
1. Match on a port range:
-------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\
action drop
$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
eth_type ipv4
ip_proto tcp
dst_port range 20-30
skip_hw
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 1 ref 1 bind 1 installed 85 sec used 3 sec
Action statistics:
Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
2. Match on IP address and port range:
--------------------------------------
$ tc filter add dev enp4s0 protocol ip parent ffff:\
prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\
skip_hw action drop
$ tc -s filter show dev enp4s0 parent ffff:
filter protocol ip pref 1 flower chain 0 handle 0x2
eth_type ipv4
ip_proto tcp
dst_ip 192.168.1.1
dst_port range 100-200
skip_hw
not_in_hw
action order 1: gact action drop
random type none pass val 0
index 2 ref 1 bind 1 installed 58 sec used 2 sec
Action statistics:
Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
v3:
1. Moved new fields in UAPI enum to the end of enum.
2. Removed couple of empty lines.
v2:
Addressed Jiri's comments:
1. Added separate functions for dst and src comparisons.
2. Removed endpoint enum.
3. Added new bit TCA_FLOWER_FLAGS_RANGE to decide normal/range
lookup.
4. Cleaned up fl_lookup function.
Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
---
include/uapi/linux/pkt_cls.h | 7 ++
net/sched/cls_flower.c | 132 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 133 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 401d0c1..95d0db2 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -485,6 +485,11 @@ enum {
TCA_FLOWER_IN_HW_COUNT,
+ TCA_FLOWER_KEY_PORT_SRC_MIN, /* be16 */
+ TCA_FLOWER_KEY_PORT_SRC_MAX, /* be16 */
+ TCA_FLOWER_KEY_PORT_DST_MIN, /* be16 */
+ TCA_FLOWER_KEY_PORT_DST_MAX, /* be16 */
+
__TCA_FLOWER_MAX,
};
@@ -518,6 +523,8 @@ enum {
TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
};
+#define TCA_FLOWER_MASK_FLAGS_RANGE (1 << 0) /* Range-based match */
+
/* Match-all classifier */
enum {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9aada2d..7780106 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -55,6 +55,8 @@ struct fl_flow_key {
struct flow_dissector_key_ip ip;
struct flow_dissector_key_ip enc_ip;
struct flow_dissector_key_enc_opts enc_opts;
+ struct flow_dissector_key_ports tp_min;
+ struct flow_dissector_key_ports tp_max;
} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
struct fl_flow_mask_range {
@@ -65,6 +67,7 @@ struct fl_flow_mask_range {
struct fl_flow_mask {
struct fl_flow_key key;
struct fl_flow_mask_range range;
+ u32 flags;
struct rhash_head ht_node;
struct rhashtable ht;
struct rhashtable_params filter_ht_params;
@@ -179,13 +182,89 @@ static void fl_clear_masked_range(struct fl_flow_key *key,
memset(fl_key_get_start(key, mask), 0, fl_mask_range(mask));
}
-static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
- struct fl_flow_key *mkey)
+static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter,
+ struct fl_flow_key *key,
+ struct fl_flow_key *mkey)
+{
+ __be16 min_mask, max_mask, min_val, max_val;
+
+ min_mask = htons(filter->mask->key.tp_min.dst);
+ max_mask = htons(filter->mask->key.tp_max.dst);
+ min_val = htons(filter->key.tp_min.dst);
+ max_val = htons(filter->key.tp_max.dst);
+
+ if (min_mask && max_mask) {
+ if (htons(key->tp.dst) < min_val ||
+ htons(key->tp.dst) > max_val)
+ return false;
+
+ /* skb does not have min and max values */
+ mkey->tp_min.dst = filter->mkey.tp_min.dst;
+ mkey->tp_max.dst = filter->mkey.tp_max.dst;
+ }
+ return true;
+}
+
+static bool fl_range_port_src_cmp(struct cls_fl_filter *filter,
+ struct fl_flow_key *key,
+ struct fl_flow_key *mkey)
+{
+ __be16 min_mask, max_mask, min_val, max_val;
+
+ min_mask = htons(filter->mask->key.tp_min.src);
+ max_mask = htons(filter->mask->key.tp_max.src);
+ min_val = htons(filter->key.tp_min.src);
+ max_val = htons(filter->key.tp_max.src);
+
+ if (min_mask && max_mask) {
+ if (htons(key->tp.src) < min_val ||
+ htons(key->tp.src) > max_val)
+ return false;
+
+ /* skb does not have min and max values */
+ mkey->tp_min.src = filter->mkey.tp_min.src;
+ mkey->tp_max.src = filter->mkey.tp_max.src;
+ }
+ return true;
+}
+
+static struct cls_fl_filter *__fl_lookup(struct fl_flow_mask *mask,
+ struct fl_flow_key *mkey)
{
return rhashtable_lookup_fast(&mask->ht, fl_key_get_start(mkey, mask),
mask->filter_ht_params);
}
+static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask,
+ struct fl_flow_key *mkey,
+ struct fl_flow_key *key)
+{
+ struct cls_fl_filter *filter, *f;
+
+ list_for_each_entry_rcu(filter, &mask->filters, list) {
+ if (!fl_range_port_dst_cmp(filter, key, mkey))
+ continue;
+
+ if (!fl_range_port_src_cmp(filter, key, mkey))
+ continue;
+
+ f = __fl_lookup(mask, mkey);
+ if (f)
+ return f;
+ }
+ return NULL;
+}
+
+static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
+ struct fl_flow_key *mkey,
+ struct fl_flow_key *key)
+{
+ if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE))
+ return fl_lookup_range(mask, mkey, key);
+
+ return __fl_lookup(mask, mkey);
+}
+
static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res)
{
@@ -208,7 +287,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
fl_set_masked_key(&skb_mkey, &skb_key, mask);
- f = fl_lookup(mask, &skb_mkey);
+ f = fl_lookup(mask, &skb_mkey, &skb_key);
if (f && !tc_skip_sw(f->flags)) {
*res = f->res;
return tcf_exts_exec(skb, &f->exts, res);
@@ -909,6 +988,23 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
sizeof(key->arp.tha));
}
+ if (key->basic.ip_proto == IPPROTO_TCP ||
+ key->basic.ip_proto == IPPROTO_UDP ||
+ key->basic.ip_proto == IPPROTO_SCTP) {
+ fl_set_key_val(tb, &key->tp_min.dst,
+ TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_min.dst,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_min.dst));
+ fl_set_key_val(tb, &key->tp_max.dst,
+ TCA_FLOWER_KEY_PORT_DST_MAX, &mask->tp_max.dst,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_max.dst));
+ fl_set_key_val(tb, &key->tp_min.src,
+ TCA_FLOWER_KEY_PORT_SRC_MIN, &mask->tp_min.src,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_min.src));
+ fl_set_key_val(tb, &key->tp_max.src,
+ TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_max.src,
+ TCA_FLOWER_UNSPEC, sizeof(key->tp_max.src));
+ }
+
if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] ||
tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) {
key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS;
@@ -1026,8 +1122,7 @@ static void fl_init_dissector(struct flow_dissector *dissector,
FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6);
- FL_KEY_SET_IF_MASKED(mask, keys, cnt,
- FLOW_DISSECTOR_KEY_PORTS, tp);
+ FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_IP, ip);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
@@ -1074,6 +1169,10 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
fl_mask_copy(newmask, mask);
+ if ((newmask->key.tp_min.dst && newmask->key.tp_max.dst) ||
+ (newmask->key.tp_min.src && newmask->key.tp_max.src))
+ newmask->flags |= TCA_FLOWER_MASK_FLAGS_RANGE;
+
err = fl_init_mask_hashtable(newmask);
if (err)
goto errout_free;
@@ -1227,7 +1326,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout_idr;
if (!tc_skip_sw(fnew->flags)) {
- if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) {
+ if (!fold && __fl_lookup(fnew->mask, &fnew->mkey)) {
err = -EEXIST;
goto errout_mask;
}
@@ -1800,6 +1899,27 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
sizeof(key->arp.tha))))
goto nla_put_failure;
+ if ((key->basic.ip_proto == IPPROTO_TCP ||
+ key->basic.ip_proto == IPPROTO_UDP ||
+ key->basic.ip_proto == IPPROTO_SCTP) &&
+ (fl_dump_key_val(skb, &key->tp_min.dst,
+ TCA_FLOWER_KEY_PORT_DST_MIN,
+ &mask->tp_min.dst, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_min.dst)) ||
+ fl_dump_key_val(skb, &key->tp_max.dst,
+ TCA_FLOWER_KEY_PORT_DST_MAX,
+ &mask->tp_max.dst, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_max.dst)) ||
+ fl_dump_key_val(skb, &key->tp_min.src,
+ TCA_FLOWER_KEY_PORT_SRC_MIN,
+ &mask->tp_min.src, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_min.src)) ||
+ fl_dump_key_val(skb, &key->tp_max.src,
+ TCA_FLOWER_KEY_PORT_SRC_MAX,
+ &mask->tp_max.src, TCA_FLOWER_UNSPEC,
+ sizeof(key->tp_max.src))))
+ goto nla_put_failure;
+
if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS &&
(fl_dump_key_val(skb, &key->enc_ipv4.src,
TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src,
^ permalink raw reply related
* Re: [PATCH v4] Wait for running BPF programs when updating map-in-map
From: Greg KH @ 2018-11-10 15:22 UTC (permalink / raw)
To: Chenbo Feng
Cc: joel, Alexei Starovoitov, Daniel Colascione, Joel Fernandes,
linux-kernel, Tim Murray, netdev, Lorenzo Colitti,
Mathieu Desnoyers, Alexei Starovoitov, Daniel Borkmann, stable
In-Reply-To: <CAMOXUJ=YFEwzBdkQw8xUrQXd4ZRzX6ob0dOCCRA=uEbjvw9yEQ@mail.gmail.com>
On Fri, Nov 09, 2018 at 06:01:54PM -0800, Chenbo Feng wrote:
> Hi netdev,
>
> Could we queue up this patch to stable 4.14 and stable 4.19? I can
> provide a backport patch if needed. I checked it is a clean
> cherry-pick for 4.19 but have some minor conflict for 4.14.
What is the git commit id of the patch in Linus's tree?
thanks
greg k-h
^ permalink raw reply
* [PATCH net-next 5/6] nfp: flower: remove TC egdev offloads
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Previously, only tunnel decap rules required egdev registration for
offload in NFP. These are now supported via indirect TC block callbacks.
Remove the egdev code from NFP.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../net/ethernet/netronome/nfp/flower/main.c | 12 ---
.../net/ethernet/netronome/nfp/flower/main.h | 3 -
.../ethernet/netronome/nfp/flower/metadata.c | 1 +
.../ethernet/netronome/nfp/flower/offload.c | 79 ++++---------------
4 files changed, 17 insertions(+), 78 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index d1c3c2081461..5059110a1768 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -146,23 +146,12 @@ nfp_flower_repr_netdev_stop(struct nfp_app *app, struct nfp_repr *repr)
return nfp_flower_cmsg_portmod(repr, false, repr->netdev->mtu, false);
}
-static int
-nfp_flower_repr_netdev_init(struct nfp_app *app, struct net_device *netdev)
-{
- return tc_setup_cb_egdev_register(netdev,
- nfp_flower_setup_tc_egress_cb,
- netdev_priv(netdev));
-}
-
static void
nfp_flower_repr_netdev_clean(struct nfp_app *app, struct net_device *netdev)
{
struct nfp_repr *repr = netdev_priv(netdev);
kfree(repr->app_priv);
-
- tc_setup_cb_egdev_unregister(netdev, nfp_flower_setup_tc_egress_cb,
- netdev_priv(netdev));
}
static void
@@ -711,7 +700,6 @@ const struct nfp_app_type app_flower = {
.vnic_init = nfp_flower_vnic_init,
.vnic_clean = nfp_flower_vnic_clean,
- .repr_init = nfp_flower_repr_netdev_init,
.repr_preclean = nfp_flower_repr_netdev_preclean,
.repr_clean = nfp_flower_repr_netdev_clean,
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 8c84829ebd21..9d134aa871fc 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -207,7 +207,6 @@ struct nfp_fl_payload {
char *unmasked_data;
char *mask_data;
char *action_data;
- bool ingress_offload;
};
extern const struct rhashtable_params nfp_flower_table_params;
@@ -259,8 +258,6 @@ void nfp_tunnel_del_ipv4_off(struct nfp_app *app, __be32 ipv4);
void nfp_tunnel_add_ipv4_off(struct nfp_app *app, __be32 ipv4);
void nfp_tunnel_request_route(struct nfp_app *app, struct sk_buff *skb);
void nfp_tunnel_keep_alive(struct nfp_app *app, struct sk_buff *skb);
-int nfp_flower_setup_tc_egress_cb(enum tc_setup_type type, void *type_data,
- void *cb_priv);
void nfp_flower_lag_init(struct nfp_fl_lag *lag);
void nfp_flower_lag_cleanup(struct nfp_fl_lag *lag);
int nfp_flower_lag_reset(struct nfp_fl_lag *lag);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index 48729bf171e0..9b4711ce98f0 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -287,6 +287,7 @@ int nfp_compile_flow_metadata(struct nfp_app *app,
nfp_flow->meta.host_ctx_id = cpu_to_be32(stats_cxt);
nfp_flow->meta.host_cookie = cpu_to_be64(flow->cookie);
+ nfp_flow->ingress_dev = netdev;
new_mask_id = 0;
if (!nfp_check_mask_add(app, nfp_flow->mask_data,
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 222e1a98cf16..0e2dfbb3ef86 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -131,7 +131,6 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
struct net_device *netdev,
struct nfp_fl_key_ls *ret_key_ls,
struct tc_cls_flower_offload *flow,
- bool egress,
enum nfp_flower_tun_type *tun_type)
{
struct flow_dissector_key_basic *mask_basic = NULL;
@@ -253,9 +252,6 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
/* Ensure the ingress netdev matches the expected tun type. */
if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type))
return -EOPNOTSUPP;
- } else if (egress) {
- /* Reject non tunnel matches offloaded to egress repr. */
- return -EOPNOTSUPP;
}
if (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) {
@@ -376,7 +372,7 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
}
static struct nfp_fl_payload *
-nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
+nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer)
{
struct nfp_fl_payload *flow_pay;
@@ -400,7 +396,6 @@ nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
flow_pay->nfp_tun_ipv4_addr = 0;
flow_pay->meta.flags = 0;
- flow_pay->ingress_offload = !egress;
return flow_pay;
@@ -418,7 +413,6 @@ nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
* @app: Pointer to the APP handle
* @netdev: netdev structure.
* @flow: TC flower classifier offload structure.
- * @egress: NFP netdev is the egress.
*
* Adds a new flow to the repeated hash structure and action payload.
*
@@ -426,47 +420,33 @@ nfp_flower_allocate_new(struct nfp_fl_key_ls *key_layer, bool egress)
*/
static int
nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
- struct tc_cls_flower_offload *flow, bool egress)
+ struct tc_cls_flower_offload *flow)
{
enum nfp_flower_tun_type tun_type = NFP_FL_TUNNEL_NONE;
struct nfp_flower_priv *priv = app->priv;
struct nfp_fl_payload *flow_pay;
struct nfp_fl_key_ls *key_layer;
struct nfp_port *port = NULL;
- struct net_device *ingr_dev;
int err;
if (nfp_netdev_is_nfp_repr(netdev))
port = nfp_port_from_netdev(netdev);
- ingr_dev = egress ? NULL : netdev;
- flow_pay = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
- NFP_FL_STATS_CTX_DONT_CARE);
- if (flow_pay) {
- /* Ignore as duplicate if it has been added by different cb. */
- if (flow_pay->ingress_offload && egress)
- return 0;
- else
- return -EOPNOTSUPP;
- }
-
key_layer = kmalloc(sizeof(*key_layer), GFP_KERNEL);
if (!key_layer)
return -ENOMEM;
err = nfp_flower_calculate_key_layers(app, netdev, key_layer, flow,
- egress, &tun_type);
+ &tun_type);
if (err)
goto err_free_key_ls;
- flow_pay = nfp_flower_allocate_new(key_layer, egress);
+ flow_pay = nfp_flower_allocate_new(key_layer);
if (!flow_pay) {
err = -ENOMEM;
goto err_free_key_ls;
}
- flow_pay->ingress_dev = egress ? NULL : netdev;
-
err = nfp_flower_compile_flow_match(app, flow, key_layer, netdev,
flow_pay, tun_type);
if (err)
@@ -476,8 +456,7 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
if (err)
goto err_destroy_flow;
- err = nfp_compile_flow_metadata(app, flow, flow_pay,
- flow_pay->ingress_dev);
+ err = nfp_compile_flow_metadata(app, flow, flow_pay, netdev);
if (err)
goto err_destroy_flow;
@@ -515,7 +494,6 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
* @app: Pointer to the APP handle
* @netdev: netdev structure.
* @flow: TC flower classifier offload structure
- * @egress: Netdev is the egress dev.
*
* Removes a flow from the repeated hash structure and clears the
* action payload.
@@ -524,22 +502,20 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
*/
static int
nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
- struct tc_cls_flower_offload *flow, bool egress)
+ struct tc_cls_flower_offload *flow)
{
struct nfp_flower_priv *priv = app->priv;
struct nfp_fl_payload *nfp_flow;
struct nfp_port *port = NULL;
- struct net_device *ingr_dev;
int err;
if (nfp_netdev_is_nfp_repr(netdev))
port = nfp_port_from_netdev(netdev);
- ingr_dev = egress ? NULL : netdev;
- nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
+ nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
NFP_FL_STATS_CTX_DONT_CARE);
if (!nfp_flow)
- return egress ? 0 : -ENOENT;
+ return -ENOENT;
err = nfp_modify_flow_metadata(app, nfp_flow);
if (err)
@@ -571,7 +547,6 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
* @app: Pointer to the APP handle
* @netdev: Netdev structure.
* @flow: TC flower classifier offload structure
- * @egress: Netdev is the egress dev.
*
* Populates a flow statistics structure which which corresponds to a
* specific flow.
@@ -580,22 +555,17 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
*/
static int
nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
- struct tc_cls_flower_offload *flow, bool egress)
+ struct tc_cls_flower_offload *flow)
{
struct nfp_flower_priv *priv = app->priv;
struct nfp_fl_payload *nfp_flow;
- struct net_device *ingr_dev;
u32 ctx_id;
- ingr_dev = egress ? NULL : netdev;
- nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, ingr_dev,
+ nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
NFP_FL_STATS_CTX_DONT_CARE);
if (!nfp_flow)
return -EINVAL;
- if (nfp_flow->ingress_offload && egress)
- return 0;
-
ctx_id = be32_to_cpu(nfp_flow->meta.host_ctx_id);
spin_lock_bh(&priv->stats_lock);
@@ -612,35 +582,18 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
static int
nfp_flower_repr_offload(struct nfp_app *app, struct net_device *netdev,
- struct tc_cls_flower_offload *flower, bool egress)
+ struct tc_cls_flower_offload *flower)
{
if (!eth_proto_is_802_3(flower->common.protocol))
return -EOPNOTSUPP;
switch (flower->command) {
case TC_CLSFLOWER_REPLACE:
- return nfp_flower_add_offload(app, netdev, flower, egress);
+ return nfp_flower_add_offload(app, netdev, flower);
case TC_CLSFLOWER_DESTROY:
- return nfp_flower_del_offload(app, netdev, flower, egress);
+ return nfp_flower_del_offload(app, netdev, flower);
case TC_CLSFLOWER_STATS:
- return nfp_flower_get_stats(app, netdev, flower, egress);
- default:
- return -EOPNOTSUPP;
- }
-}
-
-int nfp_flower_setup_tc_egress_cb(enum tc_setup_type type, void *type_data,
- void *cb_priv)
-{
- struct nfp_repr *repr = cb_priv;
-
- if (!tc_cls_can_offload_and_chain0(repr->netdev, type_data))
- return -EOPNOTSUPP;
-
- switch (type) {
- case TC_SETUP_CLSFLOWER:
- return nfp_flower_repr_offload(repr->app, repr->netdev,
- type_data, true);
+ return nfp_flower_get_stats(app, netdev, flower);
default:
return -EOPNOTSUPP;
}
@@ -657,7 +610,7 @@ static int nfp_flower_setup_tc_block_cb(enum tc_setup_type type,
switch (type) {
case TC_SETUP_CLSFLOWER:
return nfp_flower_repr_offload(repr->app, repr->netdev,
- type_data, false);
+ type_data);
default:
return -EOPNOTSUPP;
}
@@ -732,7 +685,7 @@ static int nfp_flower_setup_indr_block_cb(enum tc_setup_type type,
switch (type) {
case TC_SETUP_CLSFLOWER:
return nfp_flower_repr_offload(priv->app, priv->netdev,
- type_data, false);
+ type_data);
default:
return -EOPNOTSUPP;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/6] nfp: flower: offload tunnel decap rules via indirect TC blocks
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Previously, TC block tunnel decap rules were only offloaded when a
callback was triggered through registration of the rules egress device.
This meant that the driver had no access to the ingress netdev and so
could not verify it was the same tunnel type that the rule implied.
Register tunnel devices for indirect TC block offloads in NFP, giving
access to new rules based on the ingress device rather than egress. Use
this to verify the netdev type of VXLAN and Geneve based rules and offload
the rules to HW if applicable.
Tunnel registration is done via a netdev notifier. On notifier
registration, this is triggered for already existing netdevs. This means
that NFP can register for offloads from devices that exist before it is
loaded (filter rules will be replayed from the TC core). Similarly, on
notifier unregister, a call is triggered for each currently active netdev.
This allows the driver to unregister any indirect block callbacks that may
still be active.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
.../net/ethernet/netronome/nfp/flower/main.c | 6 +
.../net/ethernet/netronome/nfp/flower/main.h | 5 +
.../ethernet/netronome/nfp/flower/offload.c | 137 +++++++++++++++++-
3 files changed, 144 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c
index 2ad00773750f..d1c3c2081461 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
@@ -568,6 +568,8 @@ static int nfp_flower_init(struct nfp_app *app)
goto err_cleanup_metadata;
}
+ INIT_LIST_HEAD(&app_priv->indr_block_cb_priv);
+
return 0;
err_cleanup_metadata:
@@ -684,6 +686,10 @@ nfp_flower_netdev_event(struct nfp_app *app, struct net_device *netdev,
return ret;
}
+ ret = nfp_flower_reg_indir_block_handler(app, netdev, event);
+ if (ret & NOTIFY_STOP_MASK)
+ return ret;
+
return nfp_tunnel_mac_event_handler(app, netdev, event, ptr);
}
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 4a2b1a915131..8c84829ebd21 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -130,6 +130,7 @@ struct nfp_fl_lag {
* @reify_wait_queue: wait queue for repr reify response counting
* @mtu_conf: Configuration of repr MTU value
* @nfp_lag: Link aggregation data block
+ * @indr_block_cb_priv: List of priv data passed to indirect block cbs
*/
struct nfp_flower_priv {
struct nfp_app *app;
@@ -162,6 +163,7 @@ struct nfp_flower_priv {
wait_queue_head_t reify_wait_queue;
struct nfp_mtu_conf mtu_conf;
struct nfp_fl_lag nfp_lag;
+ struct list_head indr_block_cb_priv;
};
/**
@@ -271,5 +273,8 @@ int nfp_flower_lag_populate_pre_action(struct nfp_app *app,
struct nfp_fl_pre_lag *pre_act);
int nfp_flower_lag_get_output_id(struct nfp_app *app,
struct net_device *master);
+int nfp_flower_reg_indir_block_handler(struct nfp_app *app,
+ struct net_device *netdev,
+ unsigned long event);
#endif
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 2c32edfc1a9d..222e1a98cf16 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -128,6 +128,7 @@ nfp_flower_calc_opt_layer(struct flow_dissector_key_enc_opts *enc_opts,
static int
nfp_flower_calculate_key_layers(struct nfp_app *app,
+ struct net_device *netdev,
struct nfp_fl_key_ls *ret_key_ls,
struct tc_cls_flower_offload *flow,
bool egress,
@@ -186,8 +187,6 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
skb_flow_dissector_target(flow->dissector,
FLOW_DISSECTOR_KEY_ENC_CONTROL,
flow->key);
- if (!egress)
- return -EOPNOTSUPP;
if (mask_enc_ctl->addr_type != 0xffff ||
enc_ctl->addr_type != FLOW_DISSECTOR_KEY_IPV4_ADDRS)
@@ -250,6 +249,10 @@ nfp_flower_calculate_key_layers(struct nfp_app *app,
default:
return -EOPNOTSUPP;
}
+
+ /* Ensure the ingress netdev matches the expected tun type. */
+ if (!nfp_fl_netdev_is_tunnel_type(netdev, *tun_type))
+ return -EOPNOTSUPP;
} else if (egress) {
/* Reject non tunnel matches offloaded to egress repr. */
return -EOPNOTSUPP;
@@ -451,8 +454,8 @@ nfp_flower_add_offload(struct nfp_app *app, struct net_device *netdev,
if (!key_layer)
return -ENOMEM;
- err = nfp_flower_calculate_key_layers(app, key_layer, flow, egress,
- &tun_type);
+ err = nfp_flower_calculate_key_layers(app, netdev, key_layer, flow,
+ egress, &tun_type);
if (err)
goto err_free_key_ls;
@@ -693,3 +696,129 @@ int nfp_flower_setup_tc(struct nfp_app *app, struct net_device *netdev,
return -EOPNOTSUPP;
}
}
+
+struct nfp_flower_indr_block_cb_priv {
+ struct net_device *netdev;
+ struct nfp_app *app;
+ struct list_head list;
+};
+
+static struct nfp_flower_indr_block_cb_priv *
+nfp_flower_indr_block_cb_priv_lookup(struct nfp_app *app,
+ struct net_device *netdev)
+{
+ struct nfp_flower_indr_block_cb_priv *cb_priv;
+ struct nfp_flower_priv *priv = app->priv;
+
+ /* All callback list access should be protected by RTNL. */
+ ASSERT_RTNL();
+
+ list_for_each_entry(cb_priv, &priv->indr_block_cb_priv, list)
+ if (cb_priv->netdev == netdev)
+ return cb_priv;
+
+ return NULL;
+}
+
+static int nfp_flower_setup_indr_block_cb(enum tc_setup_type type,
+ void *type_data, void *cb_priv)
+{
+ struct nfp_flower_indr_block_cb_priv *priv = cb_priv;
+ struct tc_cls_flower_offload *flower = type_data;
+
+ if (flower->common.chain_index)
+ return -EOPNOTSUPP;
+
+ switch (type) {
+ case TC_SETUP_CLSFLOWER:
+ return nfp_flower_repr_offload(priv->app, priv->netdev,
+ type_data, false);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int
+nfp_flower_setup_indr_tc_block(struct net_device *netdev, struct nfp_app *app,
+ struct tc_block_offload *f)
+{
+ struct nfp_flower_indr_block_cb_priv *cb_priv;
+ struct nfp_flower_priv *priv = app->priv;
+ int err;
+
+ if (f->binder_type != TCF_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
+ return -EOPNOTSUPP;
+
+ switch (f->command) {
+ case TC_BLOCK_BIND:
+ cb_priv = kmalloc(sizeof(*cb_priv), GFP_KERNEL);
+ if (!cb_priv)
+ return -ENOMEM;
+
+ cb_priv->netdev = netdev;
+ cb_priv->app = app;
+ list_add(&cb_priv->list, &priv->indr_block_cb_priv);
+
+ err = tcf_block_cb_register(f->block,
+ nfp_flower_setup_indr_block_cb,
+ netdev, cb_priv, f->extack);
+ if (err) {
+ list_del(&cb_priv->list);
+ kfree(cb_priv);
+ }
+
+ return err;
+ case TC_BLOCK_UNBIND:
+ tcf_block_cb_unregister(f->block,
+ nfp_flower_setup_indr_block_cb, netdev);
+ cb_priv = nfp_flower_indr_block_cb_priv_lookup(app, netdev);
+ if (cb_priv) {
+ list_del(&cb_priv->list);
+ kfree(cb_priv);
+ }
+
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+
+static int
+nfp_flower_indr_setup_tc_cb(struct net_device *netdev, void *cb_priv,
+ enum tc_setup_type type, void *type_data)
+{
+ switch (type) {
+ case TC_SETUP_BLOCK:
+ return nfp_flower_setup_indr_tc_block(netdev, cb_priv,
+ type_data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+int nfp_flower_reg_indir_block_handler(struct nfp_app *app,
+ struct net_device *netdev,
+ unsigned long event)
+{
+ int err;
+
+ if (!nfp_fl_is_netdev_to_offload(netdev))
+ return NOTIFY_OK;
+
+ if (event == NETDEV_REGISTER) {
+ err = __tc_indr_block_cb_register(netdev, app,
+ nfp_flower_indr_setup_tc_cb,
+ netdev);
+ if (err)
+ nfp_flower_cmsg_warn(app,
+ "Indirect block reg failed - %s\n",
+ netdev->name);
+ } else if (event == NETDEV_UNREGISTER) {
+ __tc_indr_block_cb_unregister(netdev,
+ nfp_flower_indr_setup_tc_cb,
+ netdev);
+ }
+
+ return NOTIFY_OK;
+}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 0/6] net: sched: indirect tc block cb registration
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, Jakub Kicinski
John says:
This patchset introduces an alternative to egdev offload by allowing a
driver to register for block updates when an external device (e.g. tunnel
netdev) is bound to a TC block. Drivers can track new netdevs or register
to existing ones to receive information on such events. Based on this,
they may register for block offload rules using already existing
functions.
The patchset also implements this new indirect block registration in the
NFP driver to allow the offloading of tunnel rules. The use of egdev
offload (which is currently only used for tunnel offload) is subsequently
removed.
RFC v2 -> PATCH
- removed embedded tracking function from indir block register (now up to
driver to clean up after itself)
- refactored NFP code due to recent submissions
- removed priv list clean function in NFP (list should be cleared by
indirect block unregisters)
RFC v1->v2:
- free allocated owner struct in block_owner_clean function
- add geneve type helper function
- move test stub in NFP (v1 patch 2) to full tunnel offload
implementation via indirect blocks (v2 patches 3-8)
John Hurley (6):
net: sched: register callbacks for indirect tc block binds
nfp: flower: allow non repr netdev offload
nfp: flower: increase scope of netdev checking functions
nfp: flower: offload tunnel decap rules via indirect TC blocks
nfp: flower: remove TC egdev offloads
nfp: flower: remove unnecessary code in flow lookup
.../ethernet/netronome/nfp/flower/action.c | 28 +-
.../net/ethernet/netronome/nfp/flower/cmsg.h | 27 ++
.../net/ethernet/netronome/nfp/flower/main.c | 18 +-
.../net/ethernet/netronome/nfp/flower/main.h | 14 +-
.../net/ethernet/netronome/nfp/flower/match.c | 38 +--
.../ethernet/netronome/nfp/flower/metadata.c | 12 +-
.../ethernet/netronome/nfp/flower/offload.c | 243 +++++++++++------
.../netronome/nfp/flower/tunnel_conf.c | 19 +-
include/net/pkt_cls.h | 34 +++
include/net/sch_generic.h | 3 +
net/sched/cls_api.c | 256 +++++++++++++++++-
11 files changed, 531 insertions(+), 161 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Masami Hiramatsu @ 2018-11-10 15:31 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Aleksa Sarai, Steven Rostedt, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Jonathan Corbet, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
Brendan Gregg, Christian Brauner, netdev, linux-doc, linux-kernel
In-Reply-To: <20181109150629.wpedwxsgbftkl3ab@mikami>
On Sat, 10 Nov 2018 02:06:29 +1100
Aleksa Sarai <asarai@suse.de> wrote:
> On 2018-11-09, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> > > index ee696efec99f..c4dfafd43e11 100644
> > > --- a/arch/x86/include/asm/ptrace.h
> > > +++ b/arch/x86/include/asm/ptrace.h
> > > @@ -172,6 +172,7 @@ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
> > > return regs->sp;
> > > }
> > > #endif
> > > +#define stack_addr(regs) ((unsigned long *) kernel_stack_pointer(regs))
> >
> > No, you should use kernel_stack_pointer(regs) itself instead of stack_addr().
> >
> > >
> > > #define GET_IP(regs) ((regs)->ip)
> > > #define GET_FP(regs) ((regs)->bp)
> > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> > > index b0d1e81c96bb..eb4da885020c 100644
> > > --- a/arch/x86/kernel/kprobes/core.c
> > > +++ b/arch/x86/kernel/kprobes/core.c
> > > @@ -69,8 +69,6 @@
> > > DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> > > DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> > >
> > > -#define stack_addr(regs) ((unsigned long *)kernel_stack_pointer(regs))
> >
> > I don't like keeping this meaningless macro... this should be replaced with generic
> > kernel_stack_pointer() macro.
>
> Sure. This patch was just an example -- I can remove stack_addr() all
> over.
>
> > > - if (regs)
> > > - save_stack_address(trace, regs->ip, nosched);
> > > + if (regs) {
> > > + /* XXX: Currently broken -- stack_addr(regs) doesn't match entry. */
> > > + addr = regs->ip;
> >
> > Since this part is for storing regs->ip as a top of call-stack, this
> > seems correct code. Stack unwind will be done next block.
>
> This comment was referring to the usage of stack_addr(). stack_addr()
> doesn't give you the right result (it isn't the address of the return
> address -- it's slightly wrong). This is the main issue I was having --
> am I doing something wrong here?
Of course stack_addr() actually just returns where the stack is. It should
not return address, but maybe a return address from this event happens.
Note that the "regs != NULL" means you will be in the interrupt handler
and it will be returned to the regs->ip.
> > > + //addr = ftrace_graph_ret_addr(current, &state.graph_idx, addr, stack_addr(regs));
> >
> > so func graph return trampoline address will be shown only when unwinding stack entries.
> > I mean func-graph tracer is not used as an event, so it never kicks stackdump.
>
> Just to make sure I understand what you're saying -- func-graph trace
> will never actually call __ftrace_stack_trace? Because if it does, then
> this code will be necessary (and then I'm a bit confused why the
> unwinder has func-graph trace code -- if stack traces are never taken
> under func-graph then the code in the unwinder is not necessary)
You seems misunderstanding. Even if this is not called from func-graph
tracer, the stack entries are already replaced with func-graph trampoline.
However, regs->ip (IRQ return address) is never replaced by the func-graph
trampoline.
> My reason for commenting this out is because at this point "state" isn't
> initialised and thus .graph_idx would not be correctly handled during
> unwind (and it's the same reason I commented it out later).
OK, but anyway, I think we don't need it.
> > > + addr = kretprobe_ret_addr(current, addr, stack_addr(regs));
> >
> > But since kretprobe will be an event, which can kick the stackdump.
> > BTW, from kretprobe, regs->ip should always be the trampoline handler,
> > see arch/x86/kernel/kprobes/core.c:772 :-)
> > So it must be fixed always.
>
> Right, but kretprobe_ret_addr() is returning the *original* return
> address (and we need to do an (addr == kretprobe_trampoline)). The
> real problem is that stack_addr(regs) isn't the same as it is during
> kretprobe setup (but kretprobe_ret_addr() works everywhere else).
I think stack_addr(regs) should be same when this is called from kretprobe
handler context. Otherwise, yes, it is not same, but in that case, regs->ip
is not kretprobe_trampoline too.
If you find kretprobe_trampoline on the "stack", of course it's address should be
same as it is during kretprobe setup, but if you find kretprobe_trampoline on the
regs->ip, that should always happen on kretprobe handler context. Otherwise,
some critical violation happens on kretprobe_trampoline. In that case, we should
dump the kretprobe_trampoline address itself, should not recover it.
> > > @@ -1856,6 +1870,41 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
> > > }
> > > NOKPROBE_SYMBOL(pre_handler_kretprobe);
> > >
> > > +unsigned long kretprobe_ret_addr(struct task_struct *tsk, unsigned long ret,
> > > + unsigned long *retp)
> > > +{
> > > + struct kretprobe_instance *ri;
> > > + unsigned long flags = 0;
> > > + struct hlist_head *head;
> > > + bool need_lock;
> > > +
> > > + if (likely(ret != (unsigned long) &kretprobe_trampoline))
> > > + return ret;
> > > +
> > > + need_lock = !kretprobe_hash_is_locked(tsk);
> > > + if (WARN_ON(need_lock))
> > > + kretprobe_hash_lock(tsk, &head, &flags);
> > > + else
> > > + head = kretprobe_inst_table_head(tsk);
> >
> > This may not work unless this is called from the kretprobe handler context,
> > since if we are out of kretprobe handler context, another CPU can lock the
> > hash table and it can be detected by kretprobe_hash_is_locked();.
>
> Yeah, I noticed this as well when writing it (but needed a quick impl
> that I could test). I will fix this, thanks!
>
> By is_kretprobe_handler_context() I imagine you are referring to
> checking is_kretprobe(current_kprobe())?
yes, that's correct :)
Thank you,
>
> > So, we should check we are in the kretprobe handler context if tsk == current,
> > if not, we definately can lock the hash lock without any warning. This can
> > be something like;
> >
> > if (is_kretprobe_handler_context()) {
> > // kretprobe_hash_lock(current == tsk) has been locked by caller
> > if (tsk != current && kretprobe_hash(tsk) != kretprobe_hash(current))
> > // the hash of tsk and current can be same.
> > need_lock = true;
> > } else
> > // we should take a lock for tsk.
> > need_lock = true;
>
> --
> Aleksa Sarai
> Senior Software Engineer (Containers)
> SUSE Linux GmbH
> <https://www.cyphar.com/>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH net-next 6/6] nfp: flower: remove unnecessary code in flow lookup
From: Jakub Kicinski @ 2018-11-10 5:21 UTC (permalink / raw)
To: davem
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley
In-Reply-To: <20181110052131.3306-1-jakub.kicinski@netronome.com>
From: John Hurley <john.hurley@netronome.com>
Recent changes to NFP mean that stats updates from fw to driver no longer
require a flow lookup and (because egdev offload has been removed) the
ingress netdev for a lookup is now always known.
Remove obsolete code in a flow lookup that matches on host context and
that allows for a netdev to be NULL.
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/netronome/nfp/flower/main.h | 3 +--
drivers/net/ethernet/netronome/nfp/flower/metadata.c | 11 +++--------
drivers/net/ethernet/netronome/nfp/flower/offload.c | 6 ++----
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h
index 9d134aa871fc..b858bac47621 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/main.h
+++ b/drivers/net/ethernet/netronome/nfp/flower/main.h
@@ -20,7 +20,6 @@ struct nfp_fl_pre_lag;
struct net_device;
struct nfp_app;
-#define NFP_FL_STATS_CTX_DONT_CARE cpu_to_be32(0xffffffff)
#define NFP_FL_STATS_ELEM_RS FIELD_SIZEOF(struct nfp_fl_stats_id, \
init_unalloc)
#define NFP_FLOWER_MASK_ENTRY_RS 256
@@ -242,7 +241,7 @@ int nfp_modify_flow_metadata(struct nfp_app *app,
struct nfp_fl_payload *
nfp_flower_search_fl_table(struct nfp_app *app, unsigned long tc_flower_cookie,
- struct net_device *netdev, __be32 host_ctx);
+ struct net_device *netdev);
struct nfp_fl_payload *
nfp_flower_remove_fl_table(struct nfp_app *app, unsigned long tc_flower_cookie);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/metadata.c b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
index 9b4711ce98f0..573a4400a26c 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/metadata.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/metadata.c
@@ -21,7 +21,6 @@ struct nfp_mask_id_table {
struct nfp_fl_flow_table_cmp_arg {
struct net_device *netdev;
unsigned long cookie;
- __be32 host_ctx;
};
static int nfp_release_stats_entry(struct nfp_app *app, u32 stats_context_id)
@@ -76,14 +75,13 @@ static int nfp_get_stats_entry(struct nfp_app *app, u32 *stats_context_id)
/* Must be called with either RTNL or rcu_read_lock */
struct nfp_fl_payload *
nfp_flower_search_fl_table(struct nfp_app *app, unsigned long tc_flower_cookie,
- struct net_device *netdev, __be32 host_ctx)
+ struct net_device *netdev)
{
struct nfp_fl_flow_table_cmp_arg flower_cmp_arg;
struct nfp_flower_priv *priv = app->priv;
flower_cmp_arg.netdev = netdev;
flower_cmp_arg.cookie = tc_flower_cookie;
- flower_cmp_arg.host_ctx = host_ctx;
return rhashtable_lookup_fast(&priv->flow_table, &flower_cmp_arg,
nfp_flower_table_params);
@@ -307,8 +305,7 @@ int nfp_compile_flow_metadata(struct nfp_app *app,
priv->stats[stats_cxt].bytes = 0;
priv->stats[stats_cxt].used = jiffies;
- check_entry = nfp_flower_search_fl_table(app, flow->cookie, netdev,
- NFP_FL_STATS_CTX_DONT_CARE);
+ check_entry = nfp_flower_search_fl_table(app, flow->cookie, netdev);
if (check_entry) {
if (nfp_release_stats_entry(app, stats_cxt))
return -EINVAL;
@@ -353,9 +350,7 @@ static int nfp_fl_obj_cmpfn(struct rhashtable_compare_arg *arg,
const struct nfp_fl_flow_table_cmp_arg *cmp_arg = arg->key;
const struct nfp_fl_payload *flow_entry = obj;
- if ((!cmp_arg->netdev || flow_entry->ingress_dev == cmp_arg->netdev) &&
- (cmp_arg->host_ctx == NFP_FL_STATS_CTX_DONT_CARE ||
- flow_entry->meta.host_ctx_id == cmp_arg->host_ctx))
+ if (flow_entry->ingress_dev == cmp_arg->netdev)
return flow_entry->tc_flower_cookie != cmp_arg->cookie;
return 1;
diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 0e2dfbb3ef86..545d94168874 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -512,8 +512,7 @@ nfp_flower_del_offload(struct nfp_app *app, struct net_device *netdev,
if (nfp_netdev_is_nfp_repr(netdev))
port = nfp_port_from_netdev(netdev);
- nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
- NFP_FL_STATS_CTX_DONT_CARE);
+ nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev);
if (!nfp_flow)
return -ENOENT;
@@ -561,8 +560,7 @@ nfp_flower_get_stats(struct nfp_app *app, struct net_device *netdev,
struct nfp_fl_payload *nfp_flow;
u32 ctx_id;
- nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev,
- NFP_FL_STATS_CTX_DONT_CARE);
+ nfp_flow = nfp_flower_search_fl_table(app, flow->cookie, netdev);
if (!nfp_flow)
return -EINVAL;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] net: sched: cls_flower: validate nested enc_opts_policy to avoid build warning
From: David Miller @ 2018-11-10 4:40 UTC (permalink / raw)
To: jakub.kicinski; +Cc: oss-drivers, netdev
In-Reply-To: <20181109224122.29609-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 9 Nov 2018 14:41:22 -0800
> TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
> currently contain further nested attributes, which are parsed by
> hand, so the policy is never actually used. Add the validation
> anyway to avoid potential bugs when other attributes are added
> and to make the attribute structure slightly more clear. Validation
> will also set extact to point to bad attribute on error.
>
> Fixes: 0a6e77784f49 ("net/sched: allow flower to match tunnel options")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Acked-by: Simon Horman <simon.horman@netronome.com>
If this fixes a build warning, please include the build warning
message in your commit log.
Thanks!
^ permalink raw reply
* Re: [PATCH net] net: sched: cls_flower: validate nested enc_opts_policy to avoid build warning
From: Jakub Kicinski @ 2018-11-10 4:58 UTC (permalink / raw)
To: David Miller; +Cc: oss-drivers, netdev
In-Reply-To: <20181109.204025.1856075959976079971.davem@davemloft.net>
On Fri, 09 Nov 2018 20:40:25 -0800 (PST), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Fri, 9 Nov 2018 14:41:22 -0800
>
> > TCA_FLOWER_KEY_ENC_OPTS and TCA_FLOWER_KEY_ENC_OPTS_MASK can only
> > currently contain further nested attributes, which are parsed by
> > hand, so the policy is never actually used. Add the validation
> > anyway to avoid potential bugs when other attributes are added
> > and to make the attribute structure slightly more clear. Validation
> > will also set extact to point to bad attribute on error.
> >
> > Fixes: 0a6e77784f49 ("net/sched: allow flower to match tunnel options")
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Acked-by: Simon Horman <simon.horman@netronome.com>
>
> If this fixes a build warning, please include the build warning
> message in your commit log.
>
> Thanks!
Ah, sorry, it's a W=1 warning, which should have been mentioned, too.
I'll repost shortly!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox