Re: [PATCH net] bridge: cfm: do not reschedule TX work when interval is zero
Xiang Mei <[email protected]> Sat, 4 Apr 2026 17:04:59 -0700
| Newsgroups | dev.linux.lists.bridge,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CAPpSM+TV89o-aj7SVsAWPcVVSSte5SENe8f4tGe+Y23CHza+iA@mail.gmail.com> |
On Fri, Mar 27, 2026 at 4:21 AM Simon Horman <[email protected]> wrote: > > On Wed, Mar 25, 2026 at 08:19:57PM -0700, Xiang Mei wrote: > > ccm_tx_work_expired() uses interval_to_us() to convert the configured > > exp_interval enum into a microsecond delay, then passes it to > > queue_delayed_work() to schedule the next iteration. The ccm_tx_dwork > > callback re-arms the same delayed_work struct at the end of each > > invocation, forming a repeating timer. > > > > interval_to_us() returns 0 for BR_CFM_CCM_INTERVAL_NONE and any > > out-of-range enum value. When this 0 is passed to queue_delayed_work() > > as the delay, the work item fires immediately and re-arms itself with > > zero delay again, creating an infinite tight loop. Each iteration > > allocates an skb via ccm_frame_build() and queues it for transmission. > > The skbs pile up faster than the network stack can free them because the > > worker never yields the CPU, rapidly exhausting all kernel memory until > > OOM deadlock panic. > > > > Since CC config and CCM TX are independent netlink commands that can be > > issued in any order, there is no single configuration entry point where > > rejecting interval=0 would cover all cases. > > > > Fix this by checking the interval at the start of ccm_tx_work_expired() > > and stopping transmission immediately if it is zero. Set period to 0 so > > that br_cfm_cc_ccm_tx() correctly sees transmission as stopped and can > > restart it later if a valid interval is configured. This also avoids > > transmitting a CCM frame with an invalid interval value. > > Hi, > > I think that the principle should be that code that doesn't need > to be in the datapath shouldn't be in the datapath. > > So, with that in mind, I think it would be better to set a lower bound on > exp_interval when the mep is: > > a) Created. It looks like that happens in br_cfm_mep_create > b) Configured. It looks like that can be done by setting a policy on the > minimum value of IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL > Thanks for the suggestion. You are right, it's better to move the check out of the datapath. V2 has been sent, which blocks the invalid interval in creation/configuration. > > > > Fixes: a806ad8ee2aa ("bridge: cfm: Kernel space implementation of CFM. CCM frame TX added.") > > Reported-by: Weiming Shi <[email protected]> > > Signed-off-by: Xiang Mei <[email protected]> > > ...