DHCP Agent's iptables CHECKSUM rule causes skb_warn_bad_offload kernel

Bug #1878719 reported by Arjun Baindur
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Expired
Undecided
Unassigned

Bug Description

We are hitting this kernel issue due to a DHCP agent CHECKSUM rule that is probably obsolete/not needed: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1840619

Upgrading the kernel is one workaround, but more disruptive, especially since still using CentOS7, and kernel fix only made it into 4.19. We should just remove this rule altogether. As per the kernel issue:

"The changes are limited only to users which have CHECKSUM rules enabled in their iptables configs. Openstack commonly configures such rules on deployment, even though they are not necessary, as almost all packets have their checksum calculated by NICs these days, and CHECKSUM is only around to service old dhcp clients which would discard UDP packets with empty checksums.

This commit was selected for upstream -stable 4.18.13, and has made its way into bionic 4.15.0-58.64 by LP #1836426. There have been no reported problems and those kernels would have had sufficient testing with Openstack and its configured iptables rules.

If any users are affected by regression, then they can simply delete any CHECKSUM entries in their iptables configs."

I can see the metadata agent's CHECKSUM rule was alreayd removed last year: https://github.com/openstack/neutron/commit/04e995be9898ceaa009344509dc16ca7f589d814

Is there any reason the DHCP agent's was not? Is it safe to just remove this function and where it is invoked from altogether?

https://github.com/openstack/neutron/blob/master/neutron/agent/linux/dhcp.py#L1739
https://github.com/openstack/neutron/blob/cb55643a0695ebc5b41f50f6edb1546bcc676b71/neutron/agent/linux/dhcp.py#L1691

Revision history for this message
Arjun Baindur (abaindur) wrote :

"as almost all packets have their checksum calculated by NICs these days, and CHECKSUM is only around to service old dhcp clients which would discard UDP packets with empty checksums."

if we are worried about legacy VMs and NICs, perhaps we can add a config option for this?

Revision history for this message
Brian Haley (brian-haley) wrote :

The bug you mention references TCP checksum rules, but the one remaining in the dhcp-agent code is for UDP checksum rules. Can you paste the kernel trace you're seeing?

Changed in neutron:
status: New → Incomplete
Revision history for this message
Arjun Baindur (abaindur) wrote :
Download full text (3.4 KiB)

Hi Brian, yes, per the trace we are seeing it in TCP code. But I didn't see anything in the kernel bug about it only being for TCP? It said:

"The CHECKSUM target does not support GSO skbs, and when a GSO skb is passed to skb_checksum_help(), it errors out and skb_warn_bad_offload() is called.

The above call trace was found in a customer environment which has an Openstack deployment, with the following sorts of iptables rules set:

-A neutron-l3-agent-POSTROUTING -o qr-+ -p tcp -m tcp --sport 9697 -j CHECKSUM --checksum-fill
-A neutron-dhcp-age-POSTROUTING -p udp -m udp --dport 68 -j CHECKSUM --checksum-fill

"

We have both those rules in our environment here. Going to upgrade to Rocky, but was wondering if we needed that DHCP CHECKSUM rule as well. Based on that explanation, is it really needed if its only purpose is for legacy DHCP clients/NICs that don't do offload and checksums?

May 6 07:32:08 ns754807 kernel: Call Trace:
May 6 07:32:08 ns754807 kernel: [<ffffffffbd77ac43>] dump_stack+0x19/0x1b
May 6 07:32:08 ns754807 kernel: [<ffffffffbd09b958>] __warn+0xd8/0x100
May 6 07:32:08 ns754807 kernel: [<ffffffffbd09b9df>] warn_slowpath_fmt+0x5f/0x80
May 6 07:32:08 ns754807 kernel: [<ffffffffbd388ee3>] ? ___ratelimit+0x93/0x110
May 6 07:32:08 ns754807 kernel: [<ffffffffbd77cfcf>] skb_warn_bad_offload+0xcd/0xda
May 6 07:32:08 ns754807 kernel: [<ffffffffbd64ca55>] skb_checksum_help+0x195/0x1b0
May 6 07:32:08 ns754807 kernel: [<ffffffffc0569079>] checksum_tg+0x29/0x30 [xt_CHECKSUM]
May 6 07:32:08 ns754807 kernel: [<ffffffffc02b86c3>] ipt_do_table+0x303/0x740 [ip_tables]
May 6 07:32:08 ns754807 kernel: [<ffffffffc02b8719>] ? ipt_do_table+0x359/0x740 [ip_tables]
May 6 07:32:08 ns754807 kernel: [<ffffffffbd0a4787>] ? local_bh_enable+0x17/0x20
May 6 07:32:08 ns754807 kernel: [<ffffffffc02b8719>] ? ipt_do_table+0x359/0x740 [ip_tables]
May 6 07:32:08 ns754807 kernel: [<ffffffffc0564043>] iptable_mangle_hook+0x43/0x130 [iptable_mangle]
May 6 07:32:08 ns754807 kernel: [<ffffffffbd690438>] nf_iterate+0x98/0xe0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd690528>] nf_hook_slow+0xa8/0x110
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6a0f7a>] ip_output+0xda/0xf0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6a0320>] ? __ip_append_data.isra.50+0xa60/0xa60
May 6 07:32:08 ns754807 kernel: [<ffffffffbd69e947>] ip_local_out_sk+0x37/0x40
May 6 07:32:08 ns754807 kernel: [<ffffffffbd69ece4>] ip_queue_xmit+0x144/0x3c0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6b9394>] tcp_transmit_skb+0x4e4/0x9e0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6b9a1a>] tcp_write_xmit+0x18a/0xd40
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6ba84e>] __tcp_push_pending_frames+0x2e/0xc0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6a890c>] tcp_push+0xec/0x120
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6ac352>] tcp_sendmsg+0xd2/0xc60
May 6 07:32:08 ns754807 kernel: [<ffffffffbd6d89c9>] inet_sendmsg+0x69/0xb0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd62f3a6>] sock_sendmsg+0xb6/0xf0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd380001>] ? hctx_flags_show+0x31/0xc0
May 6 07:32:08 ns754807 kernel: [<ffffffffbd223b3d>] ? __slab_free+0x9d/0x290
May 6 07:32:08 ns754807 kernel: [<ffffffffbd62fad1>] SY...

Read more...

Revision history for this message
Arjun Baindur (abaindur) wrote :

Looking at kernel fix also, seems like it handles UDP packets specifically, as it exits early during the UDP checksum:

https://github.com/torvalds/linux/commit/10568f6c5761db24249c610c94d6e44d5505a0ba

 switch (par->family) {
 case NFPROTO_IPV4:
  if (i4->proto == IPPROTO_UDP &&
      (i4->invflags & XT_INV_PROTO) == 0)
   return 0;
  break;

So there definitely might be a case where this needs to be handled for both TCP and UDP?

Revision history for this message
Brian Haley (brian-haley) wrote :

Hi Arjun,

I believe we still need it for UDP, at least it shouldn't be causing an issue.

The kernel trace above is in the TCP code, so is probably being caused by the TCP rule the l3-agent installed previously, can you upgrade to a version that has that removed and see if you still see the issue?

Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for neutron because there has been no activity for 60 days.]

Changed in neutron:
status: Incomplete → Expired
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.