On 9/7/26 11:22 AM, Naveen Yerramneni wrote:
>
>
>> On 4 Sep 2026, at 8:33 PM, Numan Siddique <[email protected]> wrote:
>>
>> !-------------------------------------------------------------------|
>> CAUTION: External Email
>>
>> |-------------------------------------------------------------------!
>>
>> On Thu, Sep 3, 2026 at 7:31 AM Ales Musil <[email protected]> wrote:
>>>
>>>
>>>
>>> On Wed, Sep 2, 2026 at 4:33 PM Naveen Yerramneni
>>> <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>>> On 2 Sep 2026, at 6:51 PM, Ales Musil <[email protected]> wrote:
>>>>>
>>>>> CAUTION: External Email
>>>>>
>>>>>
>>>>> On Wed, Sep 2, 2026 at 12:56 PM Naveen Yerramneni
>>>>> <[email protected]> wrote:
>>>>>
>>>>>
>>>>>> On 2 Sep 2026, at 2:31 PM, Ales Musil <[email protected]> wrote:
>>>>>>
>>>>>> CAUTION: External Email
>>>>>>
>>>>>>
>>>>>> On Wed, Sep 2, 2026 at 10:46 AM Naveen Yerramneni
>>>>>> <[email protected]> wrote:
>>>>>>
>>>>>>
>>>>>>> On 2 Sep 2026, at 12:53 PM, Ales Musil <[email protected]> wrote:
>>>>>>>
>>>>>>> CAUTION: External Email
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Sep 2, 2026 at 9:03 AM Naveen Yerramneni
>>>>>>> <[email protected]> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> On 2 Sep 2026, at 11:55 AM, Ales Musil <[email protected]> wrote:
>>>>>>>>
>>>>>>>> CAUTION: External Email
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Aug 26, 2026 at 7:16 PM Naveen Yerramneni
>>>>>>>> <[email protected]> wrote:
>>>>>>>> pinctrl enqueues PACKET_OUT and NXT_RESUME messages on rconn.txq with
>>>>>>>> no limit. A PACKET_IN storm (ARP, ND, etc.) could grow the queue
>>>>>>>> without bound and use a large amount of memory.
>>>>>>>>
>>>>>>>> Limit PACKET_IN driven PACKET_OUT and NXT_RESUME messages to 8192.
>>>>>>>> Overflows are counted by the pinctrl_drop_rconn_overflow coverage
>>>>>>>> counter. The limit does not apply to OpenFlow session control
>>>>>>>> messages,
>>>>>>>> MAC-binding buffered resumes, health-check probes, BFD and other
>>>>>>>> controller originated packets.
>>>>>>>>
>>>>>>>> Acked-by: Aditya Mehakare <[email protected]>
>>>>>>>> Assisted-by: Cursor Grok 4.6, Cursor
>>>>>>>> Signed-off-by: Naveen Yerramneni <[email protected]>
>>>>>>>> ---
>>>>>>>> Hi Naveen,
>>>>>>>
>>>>>>> Hi Ales,
>>>>>>>
>>>>>>>>
>>>>>>>> thank you for the patch. I don't think this is the right approach to
>>>>>>>> solve any issues described above. This puts hard limit on the queue
>>>>>>>> that cannot be controlled in any way by the user. Also all of the
>>>>>>>> actions except for handle_dhcpv6_reply, which we could easily fix,
>>>>>>>> have CoPP meter associated with it. So you can avoid the mentioned
>>>>>>>> issues with proper CoPP configuration.
>>>>>>>>
>>>>>>>> In general if any pinctrl action can cause control plane issues we
>>>>>>>> should
>>>>>>>> have CoPP for it, if not we should add one.
>>>>>>>
>>>>>>> Thanks for the review!
>>>>>>>
>>>>>>> I agree CoPP helps in this case.
>>>>>>>
>>>>>>> I still think the pinctrl send queue should be bounded, whether or not
>>>>>>> CoPP is enabled. CoPP is optional and off by default. If an ARP
>>>>>>> flood to a non-existent IP happens with CoPP unset, pinctrl can
>>>>>>> enqueue packets without limit and ovn-controller can run out of memory.
>>>>>>>
>>>>>>> I don't think that should be the case, even if we add another config,
>>>>>>> you will end up in the same situation as with CoPP. You will have
>>>>>>> config that can protect you, which is disabled by default so it doesn't
>>>>>>> help without configuring it first. Also, since this is a new feature, it
>>>>>>> would mean 27.03, while CoPP has been available for a while. For
>>>>>>> CoPP there is no upgrade needed or anything, just configuration.
>>>>>>
>>>>>> The proposal is to enable the pinctrl TX queue limit by default
>>>>>> (8192). I think that is a reasonable default for the majority of
>>>>>> deployments. A new Open_vSwitch:external_ids option would only let the
>>>>>> user change that value, similar to OVS controller-queue-size
>>>>>> (default 100). So the protection does not require extra configuration
>>>>>> or a CMS change.
>>>>>>
>>>>>> But that is problematic, we cannot add a default restriction to something
>>>>>> that wasn't restricted before. This applies anywhere in general, while
>>>>>> 8k might be reasonable we can't be certain it indeed is for every system.
>>>>>> What if ovn runs on small system? We might run out of memory anyway.
>>>>>> Or, on the opposite end, for a large system, we would artificially limit
>>>>>> the
>>>>>> throughput. That's why it has to be opt-in, with the default remaining
>>>>>> what
>>>>>> it was before the change.
>>>>>>
>>>>>>
>>>>>> I think we should avoid unbounded packet queues in general to avoid
>>>>>> unbounded memory use.
>>>>>>
>>>>>> This is why we have CoPP.
>>>>>
>>>>>
>>>>> CoPP controls the ingress PACKET_IN rate. If the rconn unix socket
>>>>> is not draining for any reason, PACKET_OUT still piles up on
>>>>> rconn.txq on the controller side.
>>>>>
>>>>> CoPP is directly proportional to packet-out in most of the
>>>>> cases. But that wasn't the original problem description,
>>>>> packet-in storm can be managed by CoPP. ovs-vswitchd not
>>>>> draining for whatever reason is a different problem entirely.
>>>>> In that case there is bigger problem, and sure limiting the
>>>>> queue might help to mitigate that, but it would have to apply
>>>>> to all packets that we send out of pinctrl, not just some
>>>>> chosen ones.
>>>>>
>>>>
>>>>
>>>> Hi Ales,
>>>
>>>
>>> Hi Naveen,
>>>
>>>>
>>>>
>>>> The original issue we hit was with ARP. CoPP would help in that
>>>> case.
>>>>
>>>> While analysing the issue we thought it is better to avoid unbounded
>>>> queues whether CoPP is enabled or not. The patch only limited
>>>> to packets sent in response to PACKET_IN. The goal is to restrict
>>>> all packet types. I think a bounded queue is a useful extra
>>>> safety layer.
>>>>
>>>> Shall I send a v2 that includes all packet types and makes the
>>>> limit configurable?
>>>
>>>
>>> I don't think we should add yet another config option at the moment,
>>> as I don't see it as that big of deal. You can still monitor ovs-vswitchd
>>> or ovn to figure out if ovs-vswitchd is stuck. Of course I would love to
>>> hear the opinions of other maintainers.
>>
>> Let's say we add this config knob in OVN to limit the queue. What is the
>> behaviour of ovs-vswitchd in this case ? Will it keep sending the
>> packet-ins to the controller if CoPP is not enabled ?
>>
>> Sorry if this is already answered above.
>
> Hi Numan,
>
Hi all,
> Yes, OVS-vswitchd keeps sending packet-ins if CoPP is not enabled.
> This new option is to restrict the rconn tx queue limit on the controller
> side so that
> not too many packets gets queued up in the extreme cases which can lead to
> high
> memory consumption or OOM situation.
>
So, if we're only limiting packet-in -> packet-out rconn messages, I'm
all for using CoPP for that, I really think that's the only good way of
addressing those.
Now, if the worry is packets that are originated by ovn-controller,
e.g., periodic GARPs or ARP refresh packets then _maybe_ we can discuss.
Although, the premise of the problem was:
"CoPP disabled and/or packets not covered by CoPP cannot be sent because
ovs-vswitchd is not draining its side of the rconn queue".
And I agree with Ales, the whole deployment probably has a lot of issues
if that's the case. Most likely, instead of adding a relatively
targeted new config knob, the CMS should probably just monitor rconn
statistics and alert the users of the real problem: ovs-vswitchd not
draining the rconn queue.
So, my 2c until now, I wouldn't add this new knob. I would however add
what missing CoPP support we may have.
In general, but kind of a different story, maybe we should also think of
a unified infrastructure for generating alerts from OVN components.
Regards,
Dumitru
> Thanks,
> Naveen
>
>>
>> Thanks
>> Numan
>>
>>
>>>
>>>>
>>>>
>>>> Thanks,
>>>> Naveen
>>>
>>>
>>> Regards,
>>> Ales
>>>
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Naveen
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Ales
>>>>>
>>>>> Thanks,
>>>>> Naveen
>>>>>
>>>>>>
>>>>>>>
>>>>>>> We can make this queue size configurable through new option
>>>>>>> in Open_vSwitch table (external_ids column).
>>>>>>>
>>>>>>> OVS is giving similar control through controller-queue-size option.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Naveen
>>>>>>>
>>>>>>>
>>>>>>> Regards,
>>>>>>> Ales
>>>>>>>
>>>>>>>> controller/pinctrl.c | 48 ++++++++++++++++++++++++++++++++++----------
>>>>>>>> 1 file changed, 37 insertions(+), 11 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>>>>>>>> index 216831e6e..ed903fc90 100644
>>>>>>>> --- a/controller/pinctrl.c
>>>>>>>> +++ b/controller/pinctrl.c
>>>>>>>> @@ -16,6 +16,8 @@
>>>>>>>>
>>>>>>>> #include <config.h>
>>>>>>>>
>>>>>>>> +#include <errno.h>
>>>>>>>> +
>>>>>>>> #include "pinctrl.h"
>>>>>>>>
>>>>>>>> #include "coverage.h"
>>>>>>>> @@ -172,6 +174,9 @@ static struct seq *pinctrl_handler_seq;
>>>>>>>> static struct seq *pinctrl_main_seq;
>>>>>>>> static uint64_t main_seq;
>>>>>>>>
>>>>>>>> +/* Limit of tx packets can be queued on rconn.txq. */
>>>>>>>> +#define PINCTRL_QUEUE_TX_PKT_LIMIT 8192
>>>>>>>> +
>>>>>>>> #define ARP_ND_DEF_MAX_TIMEOUT 16000
>>>>>>>>
>>>>>>>> static long long int arp_nd_max_timeout = ARP_ND_DEF_MAX_TIMEOUT;
>>>>>>>> @@ -182,6 +187,8 @@ static void *pinctrl_handler(void *arg);
>>>>>>>> struct pinctrl {
>>>>>>>> /* OpenFlow connection to the switch. */
>>>>>>>> struct rconn *swconn;
>>>>>>>> + /* Counts tx packets queued on swconn. */
>>>>>>>> + struct rconn_packet_counter *tx_pending_counter;
>>>>>>>> pthread_t pinctrl_thread;
>>>>>>>> /* Latch to destroy the 'pinctrl_thread' */
>>>>>>>> struct latch pinctrl_thread_exit;
>>>>>>>> @@ -397,6 +404,7 @@ COVERAGE_DEFINE(pinctrl_ring_full_put_fdb);
>>>>>>>> COVERAGE_DEFINE(pinctrl_drop_buffered_packets_map);
>>>>>>>> COVERAGE_DEFINE(pinctrl_drop_controller_event);
>>>>>>>> COVERAGE_DEFINE(pinctrl_drop_put_vport_binding);
>>>>>>>> +COVERAGE_DEFINE(pinctrl_drop_rconn_overflow);
>>>>>>>> COVERAGE_DEFINE(pinctrl_notify_main_thread);
>>>>>>>> COVERAGE_DEFINE(pinctrl_notify_handler_thread);
>>>>>>>> COVERAGE_DEFINE(pinctrl_total_pin_pkts);
>>>>>>>> @@ -580,6 +588,7 @@ pinctrl_init(void)
>>>>>>>> bfd_monitor_init();
>>>>>>>> init_fdb_entries();
>>>>>>>> pinctrl.swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 <<
>>>>>>>> OFP15_VERSION);
>>>>>>>> + pinctrl.tx_pending_counter = rconn_packet_counter_create();
>>>>>>>> pinctrl.mac_binding_can_timestamp = false;
>>>>>>>> pinctrl_handler_seq = seq_create();
>>>>>>>> pinctrl_main_seq = seq_create();
>>>>>>>> @@ -600,6 +609,15 @@ queue_msg(struct rconn *swconn, struct ofpbuf
>>>>>>>> *msg)
>>>>>>>> return xid;
>>>>>>>> }
>>>>>>>>
>>>>>>>> +static void
>>>>>>>> +queue_msg_with_limit(struct rconn *swconn, struct ofpbuf *msg)
>>>>>>>> +{
>>>>>>>> + if (rconn_send_with_limit(swconn, msg, pinctrl.tx_pending_counter,
>>>>>>>> + PINCTRL_QUEUE_TX_PKT_LIMIT) == EAGAIN) {
>>>>>>>> + COVERAGE_INC(pinctrl_drop_rconn_overflow);
>>>>>>>> + }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> /* Sets up 'swconn', a newly (re)connected connection to a switch. */
>>>>>>>> static void
>>>>>>>> pinctrl_setup(struct rconn *swconn)
>>>>>>>> @@ -639,7 +657,7 @@ enqueue_packet(struct rconn *swconn, enum
>>>>>>>> ofp_version version,
>>>>>>>>
>>>>>>>> match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
>>>>>>>> enum ofputil_protocol proto =
>>>>>>>> ofputil_protocol_from_ofp_version(version);
>>>>>>>> - queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>>>>>>>> + queue_msg_with_limit(swconn, ofputil_encode_packet_out(&po,
>>>>>>>> proto));
>>>>>>>> }
>>>>>>>>
>>>>>>>> static void
>>>>>>>> @@ -751,7 +769,7 @@ pinctrl_forward_pkt(struct rconn *swconn, int64_t
>>>>>>>> dp_key,
>>>>>>>> };
>>>>>>>> match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
>>>>>>>> enum ofputil_protocol proto =
>>>>>>>> ofputil_protocol_from_ofp_version(version);
>>>>>>>> - queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>>>>>>>> + queue_msg_with_limit(swconn, ofputil_encode_packet_out(&po,
>>>>>>>> proto));
>>>>>>>> ofpbuf_uninit(&ofpacts);
>>>>>>>> }
>>>>>>>>
>>>>>>>> @@ -1045,7 +1063,7 @@ pinctrl_parse_dhcpv6_advt(struct rconn *swconn,
>>>>>>>> const struct flow *ip_flow,
>>>>>>>> };
>>>>>>>> match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
>>>>>>>> enum ofputil_protocol proto =
>>>>>>>> ofputil_protocol_from_ofp_version(version);
>>>>>>>> - queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>>>>>>>> + queue_msg_with_limit(swconn, ofputil_encode_packet_out(&po,
>>>>>>>> proto));
>>>>>>>> dp_packet_uninit(&packet);
>>>>>>>> ofpbuf_uninit(&ofpacts);
>>>>>>>>
>>>>>>>> @@ -2382,7 +2400,8 @@ exit:
>>>>>>>> sv.u8_val = success;
>>>>>>>> mf_write_subfield(&dst, &sv, &pin->flow_metadata);
>>>>>>>> }
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> if (pkt_out_ptr) {
>>>>>>>> dp_packet_uninit(pkt_out_ptr);
>>>>>>>> }
>>>>>>>> @@ -2594,7 +2613,8 @@ exit:
>>>>>>>> sv.u8_val = success;
>>>>>>>> mf_write_subfield(&dst, &sv, &pin->flow_metadata);
>>>>>>>> }
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> if (pkt_out_ptr) {
>>>>>>>> dp_packet_uninit(pkt_out_ptr);
>>>>>>>> }
>>>>>>>> @@ -2934,7 +2954,8 @@ exit:
>>>>>>>> sv.u8_val = success;
>>>>>>>> mf_write_subfield(&dst, &sv, &pin->flow_metadata);
>>>>>>>> }
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> if (pkt_out_ptr) {
>>>>>>>> dp_packet_uninit(pkt_out_ptr);
>>>>>>>> }
>>>>>>>> @@ -3360,7 +3381,8 @@ exit:
>>>>>>>> sv.u8_val = success;
>>>>>>>> mf_write_subfield(&dst, &sv, &pin->flow_metadata);
>>>>>>>> }
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> dp_packet_uninit(pkt_out_ptr);
>>>>>>>> }
>>>>>>>>
>>>>>>>> @@ -3740,7 +3762,8 @@ exit:
>>>>>>>> set_from_ctrl_flag_in_pkt_metadata(pin);
>>>>>>>>
>>>>>>>> }
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> dp_packet_uninit(pkt_out_ptr);
>>>>>>>> }
>>>>>>>>
>>>>>>>> @@ -4781,6 +4804,7 @@ pinctrl_destroy(void)
>>>>>>>> pthread_join(pinctrl.pinctrl_thread, NULL);
>>>>>>>> latch_destroy(&pinctrl.pinctrl_thread_exit);
>>>>>>>> rconn_destroy(pinctrl.swconn);
>>>>>>>> + rconn_packet_counter_destroy(pinctrl.tx_pending_counter);
>>>>>>>> destroy_send_arps_nds();
>>>>>>>> destroy_ipv6_ras();
>>>>>>>> destroy_ipv6_prefixd();
>>>>>>>> @@ -6691,7 +6715,8 @@ exit:
>>>>>>>> sv.u8_val = success;
>>>>>>>> mf_write_subfield(&dst, &sv, &pin->flow_metadata);
>>>>>>>> }
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> dp_packet_uninit(pkt_out_ptr);
>>>>>>>> }
>>>>>>>>
>>>>>>>> @@ -6804,7 +6829,8 @@ pinctrl_handle_put_icmp4_inner_ip4_src(struct
>>>>>>>> rconn *swconn,
>>>>>>>> pin->packet_len = dp_packet_size(pkt_out);
>>>>>>>>
>>>>>>>> exit:
>>>>>>>> - queue_msg(swconn, ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> + queue_msg_with_limit(swconn,
>>>>>>>> + ofputil_encode_resume(pin, continuation,
>>>>>>>> proto));
>>>>>>>> if (pkt_out) {
>>>>>>>> dp_packet_delete(pkt_out);
>>>>>>>> }
>>>>>>>> @@ -9029,7 +9055,7 @@ pinctrl_split_buf_action_handler(struct rconn
>>>>>>>> *swconn, struct dp_packet *pkt,
>>>>>>>> match_set_in_port(&po.flow_metadata, OFPP_CONTROLLER);
>>>>>>>> enum ofp_version version = rconn_get_version(swconn);
>>>>>>>> enum ofputil_protocol proto =
>>>>>>>> ofputil_protocol_from_ofp_version(version);
>>>>>>>> - queue_msg(swconn, ofputil_encode_packet_out(&po, proto));
>>>>>>>> + queue_msg_with_limit(swconn, ofputil_encode_packet_out(&po,
>>>>>>>> proto));
>>>>>>>>
>>>>>>>> ofpbuf_uninit(&ofpacts);
>>>>>>>> }
>>>>>>>> --
>>>>>>>> 2.43.5
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> dev mailing list
>>>>>>>> [email protected]
>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=2PQjSDR7A28z1kXE1ptSm6X36oL_nCq1XxeEt7FkLmA&m=lq_DmPMKNcUTPolFj_QSDsyELKJJcwHXy3o8Zy20Hmsimj4pxDHRLMpj6sdKkNNE&s=lD09qlu6cLSP7oQi6ehjASaYSvf04OmdcszWvSAOT0Q&e=
>>>>>>>> [mail.openvswitch.org] [mail.openvswitch.org [mail.openvswitch.org]]
>>>>>>>> [mail.openvswitch.org [mail.openvswitch.org] [mail.openvswitch.org
>>>>>>>> [mail.openvswitch.org]]] [mail.openvswitch.org
>>>>>>>> [mail.openvswitch.org][mail.openvswitch.org [mail.openvswitch.org]]
>>>>>>>> [mail.openvswitch.org [mail.openvswitch.org] [mail.openvswitch.org
>>>>>>>> [mail.openvswitch.org]]]]
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Ales
>>>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
