> 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,
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.
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
