[ovs-dev] [PATCH v3] vswitchd: Make packet-in controller queue size configurable

Mark Michelson mmichels at redhat.com
Fri Aug 9 15:08:06 UTC 2019


Acked-by: Mark Michelson <mmichels at redhat.com>

On 8/2/19 4:29 AM, Dumitru Ceara wrote:
> The ofconn packet-in queue for packets that can't be immediately sent
> on the rconn connection was limited to 100 packets (hardcoded value).
> While increasing this limit is usually not recommended as it might
> create buffer bloat and increase latency, in scaled scenarios it is
> useful if the administrator (or CMS) can adjust the queue size.
> 
> One such situation was noticed while performing scale testing of the
> OVN IGMP functionality: triggering ~200 simultaneous IGMP reports
> was causing tail drops on the packet-in queue towards ovn-controller.
> 
> This commit adds the possibility to configure the queue size for:
> - management controller (br-int.mgmt): through the
>    other_config:controller-queue-size column of the Bridge table. This
>    value is limited to 512 as large queues definitely affect latency. If
>    not present the default value of 100 is used. This is done in order to
>    maintain the same default behavior as before the commit.
> - other controllers: through the controller_queue_size column of the
>    Controller table. This value is also limited to 512. If not present
>    the code uses the Bridge:other_config:controller-queue-size
>    configuration.
> 
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> 
> ---
> v3: Remove trailing whitespace.
> v2: Address reviewer comments (increase vswitch.vsschema version &
>      reword docs).
> ---
>   ofproto/connmgr.c          |  6 +++++-
>   ofproto/ofproto.h          |  1 +
>   vswitchd/bridge.c          | 26 ++++++++++++++++++++++++++
>   vswitchd/vswitch.ovsschema |  9 +++++++--
>   vswitchd/vswitch.xml       | 21 +++++++++++++++++++++
>   5 files changed, 60 insertions(+), 3 deletions(-)
> 
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index d975a53..51d656c 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -74,6 +74,7 @@ struct ofconn {
>       enum ofputil_packet_in_format packet_in_format;
>   
>       /* OFPT_PACKET_IN related data. */
> +    int packet_in_queue_size;
>       struct rconn_packet_counter *packet_in_counter; /* # queued on 'rconn'. */
>   #define N_SCHEDULERS 2
>       struct pinsched *schedulers[N_SCHEDULERS];
> @@ -1176,6 +1177,7 @@ ofconn_create(struct ofservice *ofservice, struct rconn *rconn,
>       ofconn_set_protocol(ofconn, OFPUTIL_P_NONE);
>       ofconn->packet_in_format = OFPUTIL_PACKET_IN_STD;
>   
> +    ofconn->packet_in_queue_size = settings->max_pktq_size;
>       ofconn->packet_in_counter = rconn_packet_counter_create();
>       ofconn->miss_send_len = (ofconn->type == OFCONN_PRIMARY
>                                ? OFP_DEFAULT_MISS_SEND_LEN
> @@ -1263,6 +1265,7 @@ ofconn_reconfigure(struct ofconn *ofconn, const struct ofproto_controller *c)
>       rconn_set_probe_interval(ofconn->rconn, probe_interval);
>   
>       ofconn->band = c->band;
> +    ofconn->packet_in_queue_size = c->max_pktq_size;
>   
>       ofconn_set_rate_limit(ofconn, c->rate_limit, c->burst_limit);
>   
> @@ -1676,7 +1679,8 @@ do_send_packet_ins(struct ofconn *ofconn, struct ovs_list *txq)
>   
>       LIST_FOR_EACH_POP (pin, list_node, txq) {
>           if (rconn_send_with_limit(ofconn->rconn, pin,
> -                                  ofconn->packet_in_counter, 100) == EAGAIN) {
> +                                  ofconn->packet_in_counter,
> +                                  ofconn->packet_in_queue_size) == EAGAIN) {
>               static struct vlog_rate_limit rll = VLOG_RATE_LIMIT_INIT(5, 5);
>   
>               VLOG_INFO_RL(&rll, "%s: dropping packet-in due to queue overflow",
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 6e4afff..b53ea03 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -234,6 +234,7 @@ struct ofproto_controller {
>                                    * be negotiated for a session. */
>   
>       /* OpenFlow packet-in rate-limiting. */
> +    int max_pktq_size;          /* Maximum number of packet-in to be queued. */
>       int rate_limit;             /* Max packet-in rate in packets per second. */
>       int burst_limit;            /* Limit on accumulating packet credits. */
>   
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 2976771..f9d17b7 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -227,6 +227,11 @@ static struct if_notifier *ifnotifier;
>   static struct seq *ifaces_changed;
>   static uint64_t last_ifaces_changed;
>   
> +/* Default/min/max packet-in queue sizes towards the controllers. */
> +#define BRIDGE_CONTROLLER_PACKET_QUEUE_DEFAULT_SIZE 100
> +#define BRIDGE_CONTROLLER_PACKET_QUEUE_MIN_SIZE 1
> +#define BRIDGE_CONTROLLER_PACKET_QUEUE_MAX_SIZE 512
> +
>   static void add_del_bridges(const struct ovsrec_open_vswitch *);
>   static void bridge_run__(void);
>   static void bridge_create(const struct ovsrec_bridge *);
> @@ -1117,6 +1122,25 @@ bridge_get_allowed_versions(struct bridge *br)
>                                            br->cfg->n_protocols);
>   }
>   
> +static int
> +bridge_get_controller_queue_size(struct bridge *br,
> +                                 struct ovsrec_controller *c)
> +{
> +    if (c && c->controller_queue_size) {
> +        return *c->controller_queue_size;
> +    }
> +
> +    int queue_size = smap_get_int(&br->cfg->other_config,
> +                                  "controller-queue-size",
> +                                  BRIDGE_CONTROLLER_PACKET_QUEUE_DEFAULT_SIZE);
> +    if (queue_size < BRIDGE_CONTROLLER_PACKET_QUEUE_MIN_SIZE ||
> +            queue_size > BRIDGE_CONTROLLER_PACKET_QUEUE_MAX_SIZE) {
> +        return BRIDGE_CONTROLLER_PACKET_QUEUE_DEFAULT_SIZE;
> +    }
> +
> +    return queue_size;
> +}
> +
>   /* Set NetFlow configuration on 'br'. */
>   static void
>   bridge_configure_netflow(struct bridge *br)
> @@ -3605,6 +3629,7 @@ bridge_configure_remotes(struct bridge *br,
>           .band = OFPROTO_OUT_OF_BAND,
>           .enable_async_msgs = true,
>           .allowed_versions = bridge_get_allowed_versions(br),
> +        .max_pktq_size = bridge_get_controller_queue_size(br, NULL),
>       };
>       shash_add_nocopy(
>           &ocs, xasprintf("punix:%s/%s.mgmt", ovs_rundir(), br->name), oc);
> @@ -3680,6 +3705,7 @@ bridge_configure_remotes(struct bridge *br,
>               .enable_async_msgs = (!c->enable_async_messages
>                                     || *c->enable_async_messages),
>               .allowed_versions = bridge_get_allowed_versions(br),
> +            .max_pktq_size = bridge_get_controller_queue_size(br, c),
>               .rate_limit = (c->controller_rate_limit
>                              ? *c->controller_rate_limit : 0),
>               .burst_limit = (c->controller_burst_limit
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index f7c6eb8..4f7ba59 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,6 +1,6 @@
>   {"name": "Open_vSwitch",
> - "version": "8.0.0",
> - "cksum": "3962141869 23978",
> + "version": "8.1.0",
> + "cksum": "3492192244 24186",
>    "tables": {
>      "Open_vSwitch": {
>        "columns": {
> @@ -575,6 +575,11 @@
>          "enable_async_messages": {
>            "type": {"key": {"type": "boolean"},
>                     "min": 0, "max": 1}},
> +       "controller_queue_size": {
> +         "type": {"key": {"type": "integer",
> +                          "minInteger": 1,
> +                          "maxInteger": 512},
> +                  "min": 0, "max": 1}},
>          "controller_rate_limit": {
>            "type": {"key": {"type": "integer",
>                             "minInteger": 100},
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 027aee2..d05d423 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -1268,6 +1268,15 @@
>           ID, the default queue is used instead.
>         </column>
>   
> +      <column name="other_config" key="controller-queue-size"
> +              type='{"type": "integer", "minInteger": 1, "maxInteger": 512}'>
> +        This sets the maximum size of the queue of packets that need to be
> +        sent to the OpenFlow management controller. The value must be less
> +        than 512. If not specified the queue size is limited to 100 packets
> +        by default. Note: increasing the queue size might have a negative
> +        impact on latency.
> +      </column>
> +
>         <column name="protocols">
>           List of OpenFlow protocols that may be used when negotiating a
>           connection with a controller.  OpenFlow 1.0, 1.1, 1.2, 1.3, 1.4, and
> @@ -5003,6 +5012,18 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>             table="Interface"/> table for ingress policing configuration.
>           </p>
>   
> +      <column name="controller_queue_size">
> +        <p>
> +           This sets the maximum size of the queue of packets that need to be
> +           sent to this OpenFlow controller. The value must be less than 512.
> +           If not specified the queue size is limited to the value set for
> +           the management controller in <ref table="Bridge"
> +           column="other_config" key="controller-queue-size"/> if present or
> +           100 packets by default. Note: increasing the queue size might
> +           have a negative impact on latency.
> +        </p>
> +      </column>
> +
>           <column name="controller_rate_limit">
>             <p>
>               The maximum rate at which the switch will forward packets to the
> 



More information about the dev mailing list