Thanks for the feedbacks. Please see the detail below: On Wed, Apr 11, 2018 at 3:37 PM, Julian Anastasov <j...@ssi.bg> wrote: [snip] >> - __IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS); >> + __IP_INC_STATS(net, skb_dst(skb)->dev, IPSTATS_MIB_INHDRERRORS); > > May be skb->dev if we want to account it to the > input device. > Yes. I'm about to make change it but see the next one.
[snip] >> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c >> b/net/netfilter/ipvs/ip_vs_xmit.c >> index 4527921..32bd3af 100644 >> --- a/net/netfilter/ipvs/ip_vs_xmit.c >> +++ b/net/netfilter/ipvs/ip_vs_xmit.c >> @@ -286,7 +286,7 @@ static inline bool decrement_ttl(struct netns_ipvs *ipvs, >> { >> if (ip_hdr(skb)->ttl <= 1) { >> /* Tell the sender its packet died... */ >> - __IP_INC_STATS(net, IPSTATS_MIB_INHDRERRORS); >> + __IP_INC_STATS(net, skb_dst(skb)->dev, >> IPSTATS_MIB_INHDRERRORS); > > At this point, skb_dst(skb) can be: > > - input route at LOCAL_IN => dst->dev is "lo", skb->dev = input_device > - output route at LOCAL_OUT => dst->dev is output_device, skb->dev = NULL > > We should see this error on LOCAL_IN but better to be > safe: use 'skb->dev ? : skb_dst(skb)->dev' instead of just > 'skb_dst(skb)->dev'. > This follows v6 implementation in the same function: #ifdef CONFIG_IP_VS_IPV6 if (skb_af == AF_INET6) { struct dst_entry *dst = skb_dst(skb); /* check and decrement ttl */ if (ipv6_hdr(skb)->hop_limit <= 1) { /* Force OUTPUT device used as source address */ skb->dev = dst->dev; icmpv6_send(skb, ICMPV6_TIME_EXCEED, ICMPV6_EXC_HOPLIMIT, 0); __IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_INHDRERRORS); return false; } /* don't propagate ttl change to cloned packets */ if (!skb_make_writable(skb, sizeof(struct ipv6hdr))) return false; ipv6_hdr(skb)->hop_limit--; } else #endif [snip] > > The patch probably has other errors, for example, > using rt->dst.dev (lo) when rt->dst.error != 0 in ip_error, > may be 'dev' should be used instead... Same also here. Examples are ip6_forward and ip6_pkt_drop. I think it's better be counted in the input device for them also. Thoughts? Regards, Stephen.