| Message ID | 1536462862-11767-2-git-send-email-laoar.shao@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net-next,1/2] tcp: show number of network segments in some SNMP counters | expand |
On Sat, Sep 8, 2018 at 8:14 PM Yafang Shao <laoar.shao@gmail.com> wrote: > > In RFC1213, the tcpInSegs is the total number of segments received. > While currently it is the total number of SKBs received. > The number of SKBs may be not equal with the numer of segments because of > GRO. > So fix this error count. > We have discussed this in the past and the consensus was it was too late to change this. IP counters have the same issue, so after your patch, we would have quite a difference between transport and network layers. Adding all these max_t(u16, 1, skb_shinfo(skb)->gso_segs)) everywhere add a cost
On Mon, Sep 10, 2018 at 2:32 AM, Eric Dumazet <edumazet@google.com> wrote: > On Sat, Sep 8, 2018 at 8:14 PM Yafang Shao <laoar.shao@gmail.com> wrote: >> >> In RFC1213, the tcpInSegs is the total number of segments received. >> While currently it is the total number of SKBs received. >> The number of SKBs may be not equal with the numer of segments because of >> GRO. >> So fix this error count. >> > > We have discussed this in the past and the consensus was it was too > late to change this. > > IP counters have the same issue, so after your patch, we would have > quite a difference between transport and network layers. > > Adding all these max_t(u16, 1, skb_shinfo(skb)->gso_segs)) everywhere add a cost May be we could give a comment here why we do it like this, otherwise it may make a misunderstanding. Thanks Yafang
diff --git a/include/net/tcp.h b/include/net/tcp.h index 770917d..66578f4 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -310,6 +310,8 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift) #define __TCP_INC_STATS(net, field) __SNMP_INC_STATS((net)->mib.tcp_statistics, field) #define TCP_DEC_STATS(net, field) SNMP_DEC_STATS((net)->mib.tcp_statistics, field) #define TCP_ADD_STATS(net, field, val) SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val) +#define __TCP_ADD_STATS(net, field, val) \ + __SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val) void tcp_tasklet_init(void); diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 23d7cb5..2b98242 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1703,7 +1703,8 @@ int tcp_v4_rcv(struct sk_buff *skb) goto discard_it; /* Count it even if it's bad */ - __TCP_INC_STATS(net, TCP_MIB_INSEGS); + __TCP_ADD_STATS(net, TCP_MIB_INSEGS, + max_t(u16, 1, skb_shinfo(skb)->gso_segs)); if (!pskb_may_pull(skb, sizeof(struct tcphdr))) goto discard_it; diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index bbf7667..8d4ef46 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1441,7 +1441,8 @@ static int tcp_v6_rcv(struct sk_buff *skb) /* * Count it even if it's bad. */ - __TCP_INC_STATS(net, TCP_MIB_INSEGS); + __TCP_ADD_STATS(net, TCP_MIB_INSEGS, + max_t(u16, 1, skb_shinfo(skb)->gso_segs)); if (!pskb_may_pull(skb, sizeof(struct tcphdr))) goto discard_it;
In RFC1213, the tcpInSegs is the total number of segments received. While currently it is the total number of SKBs received. The number of SKBs may be not equal with the numer of segments because of GRO. So fix this error count. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/net/tcp.h | 2 ++ net/ipv4/tcp_ipv4.c | 3 ++- net/ipv6/tcp_ipv6.c | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-)