| Message ID | 20241203110853.201377-6-amusil@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Numan Siddique |
| Headers | show |
| Series | Introduce the concept of transit router. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Tue, Dec 3, 2024 at 12:09 PM Ales Musil <amusil@redhat.com> wrote: > The action used for multiple split buffer scenario, when the > action wouldn't fit into the netlink buffer, can be used in > different context than just the multicast groups. Change the > naming and add generic helper so it is clearer that is indeed > the case. > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > controller/physical.c | 36 ++++++++++++++++++++++-------------- > controller/pinctrl.c | 21 +++++++++++---------- > include/ovn/actions.h | 4 ++-- > lib/actions.c | 2 +- > 4 files changed, 36 insertions(+), 27 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index adf37600d..1adc0a5f6 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -193,6 +193,27 @@ put_stack(enum mf_field_id field, struct ofpact_stack > *stack) > stack->subfield.n_bits = stack->subfield.field->n_bits; > } > > +/* Split the ofpacts buffer to prevent overflow of the > + * MAX_ACTIONS_BUFSIZE netlink buffer size supported by the kernel. > + * In order to avoid all the action buffers to be squashed together by > + * ovs, add a controller action for each configured openflow. > + */ > +static void > +put_split_buf_function(uint32_t index, uint32_t outport, uint8_t stage, > + struct ofpbuf *ofpacts) > +{ > + ovs_be32 values[2] = { > + htonl(index), > + htonl(outport) > + }; > + size_t oc_offset = > + encode_start_controller_op(ACTION_OPCODE_SPLIT_BUF_ACTION, > false, > + NX_CTLR_NO_METER, ofpacts); > + ofpbuf_put(ofpacts, values, sizeof values); > + ofpbuf_put(ofpacts, &stage, sizeof stage); > + encode_finish_controller_op(oc_offset, ofpacts); > +} > + > static const struct sbrec_port_binding * > get_localnet_port(const struct hmap *local_datapaths, int64_t tunnel_key) > { > @@ -2088,20 +2109,7 @@ mc_ofctrl_add_flow(const struct > sbrec_multicast_group *mc, > if (index == (mc->n_ports - 1)) { > ofpbuf_put(ofpacts, ofpacts_last->data, ofpacts_last->size); > } else { > - /* Split multicast groups with size greater than > - * MC_OFPACTS_MAX_MSG_SIZE in order to not overcome the > - * MAX_ACTIONS_BUFSIZE netlink buffer size supported by the > kernel. > - * In order to avoid all the action buffers to be squashed > together by > - * ovs, add a controller action for each configured openflow. > - */ > - size_t oc_offset = encode_start_controller_op( > - ACTION_OPCODE_MG_SPLIT_BUF, false, NX_CTLR_NO_METER, > ofpacts); > - ovs_be32 val = htonl(++flow_index); > - ofpbuf_put(ofpacts, &val, sizeof val); > - val = htonl(mc->tunnel_key); > - ofpbuf_put(ofpacts, &val, sizeof val); > - ofpbuf_put(ofpacts, &stage, sizeof stage); > - encode_finish_controller_op(oc_offset, ofpacts); > + put_split_buf_function(++flow_index, mc->tunnel_key, stage, > ofpacts); > } > > ofctrl_add_flow(flow_table, stage, prio, mc->header_.uuid.parts[0], > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 3fb7e2fd7..01a77bf93 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -209,7 +209,7 @@ static void send_mac_binding_buffered_pkts(struct > rconn *swconn) > > static void pinctrl_rarp_activation_strategy_handler(const struct match > *md); > > -static void pinctrl_mg_split_buff_handler( > +static void pinctrl_split_buf_action_handler( > struct rconn *swconn, struct dp_packet *pkt, > const struct match *md, struct ofpbuf *userdata); > > @@ -3772,9 +3772,9 @@ process_packet_in(struct rconn *swconn, const struct > ofp_header *msg) > ovs_mutex_unlock(&pinctrl_mutex); > break; > > - case ACTION_OPCODE_MG_SPLIT_BUF: > - pinctrl_mg_split_buff_handler(swconn, &packet, &pin.flow_metadata, > - &userdata); > + case ACTION_OPCODE_SPLIT_BUF_ACTION: > + pinctrl_split_buf_action_handler(swconn, &packet, > &pin.flow_metadata, > + &userdata); > break; > > default: > @@ -8690,8 +8690,9 @@ pinctrl_rarp_activation_strategy_handler(const > struct match *md) > } > > static void > -pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, > - const struct match *md, struct ofpbuf > *userdata) > +pinctrl_split_buf_action_handler(struct rconn *swconn, struct dp_packet > *pkt, > + const struct match *md, > + struct ofpbuf *userdata) > { > ovs_be32 *index = ofpbuf_try_pull(userdata, sizeof *index); > if (!index) { > @@ -8700,10 +8701,10 @@ pinctrl_mg_split_buff_handler(struct rconn > *swconn, struct dp_packet *pkt, > return; > } > > - ovs_be32 *mg = ofpbuf_try_pull(userdata, sizeof *mg); > - if (!mg) { > + ovs_be32 *outport = ofpbuf_try_pull(userdata, sizeof *outport); > + if (!outport) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > - VLOG_WARN_RL(&rl, "%s: missing multicast group field", __func__); > + VLOG_WARN_RL(&rl, "%s: missing outport tunnel_key field", > __func__); > return; > } > > @@ -8725,7 +8726,7 @@ pinctrl_mg_split_buff_handler(struct rconn *swconn, > struct dp_packet *pkt, > ofpact_put_set_field(&ofpacts, pkt_mark_field, &pkt_mark_value, NULL); > > put_load(ntohl(*index), MFF_REG6, 0, 32, &ofpacts); > - put_load(ntohl(*mg), MFF_LOG_OUTPORT, 0, 32, &ofpacts); > + put_load(ntohl(*outport), MFF_LOG_OUTPORT, 0, 32, &ofpacts); > > struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts); > resubmit->in_port = OFPP_CONTROLLER; > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index db7342f1d..7e0670a11 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -783,8 +783,8 @@ enum action_opcode { > /* activation_strategy_rarp() */ > ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, > > - /* multicast group split buffer action. */ > - ACTION_OPCODE_MG_SPLIT_BUF, > + /* split buffer action. */ > + ACTION_OPCODE_SPLIT_BUF_ACTION, > > /* "dhcp_relay_req_chk(relay_ip, server_ip)". > * > diff --git a/lib/actions.c b/lib/actions.c > index d5fc30b27..4a328b03d 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -1968,7 +1968,7 @@ is_paused_nested_action(enum action_opcode opcode) > case ACTION_OPCODE_HANDLE_SVC_CHECK: > case ACTION_OPCODE_BFD_MSG: > case ACTION_OPCODE_ACTIVATION_STRATEGY_RARP: > - case ACTION_OPCODE_MG_SPLIT_BUF: > + case ACTION_OPCODE_SPLIT_BUF_ACTION: > case ACTION_OPCODE_DHCP_RELAY_REQ_CHK: > case ACTION_OPCODE_DHCP_RELAY_RESP_CHK: > default: > -- > 2.47.0 > > Recheck-request: github-robot-_Build_and_Test
> The action used for multiple split buffer scenario, when the > action wouldn't fit into the netlink buffer, can be used in > different context than just the multicast groups. Change the > naming and add generic helper so it is clearer that is indeed > the case. > > Signed-off-by: Ales Musil <amusil@redhat.com> Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/physical.c | 36 ++++++++++++++++++++++-------------- > controller/pinctrl.c | 21 +++++++++++---------- > include/ovn/actions.h | 4 ++-- > lib/actions.c | 2 +- > 4 files changed, 36 insertions(+), 27 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index adf37600d..1adc0a5f6 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -193,6 +193,27 @@ put_stack(enum mf_field_id field, struct ofpact_stack *stack) > stack->subfield.n_bits = stack->subfield.field->n_bits; > } > > +/* Split the ofpacts buffer to prevent overflow of the > + * MAX_ACTIONS_BUFSIZE netlink buffer size supported by the kernel. > + * In order to avoid all the action buffers to be squashed together by > + * ovs, add a controller action for each configured openflow. > + */ > +static void > +put_split_buf_function(uint32_t index, uint32_t outport, uint8_t stage, > + struct ofpbuf *ofpacts) > +{ > + ovs_be32 values[2] = { > + htonl(index), > + htonl(outport) > + }; > + size_t oc_offset = > + encode_start_controller_op(ACTION_OPCODE_SPLIT_BUF_ACTION, false, > + NX_CTLR_NO_METER, ofpacts); > + ofpbuf_put(ofpacts, values, sizeof values); > + ofpbuf_put(ofpacts, &stage, sizeof stage); > + encode_finish_controller_op(oc_offset, ofpacts); > +} > + > static const struct sbrec_port_binding * > get_localnet_port(const struct hmap *local_datapaths, int64_t tunnel_key) > { > @@ -2088,20 +2109,7 @@ mc_ofctrl_add_flow(const struct sbrec_multicast_group *mc, > if (index == (mc->n_ports - 1)) { > ofpbuf_put(ofpacts, ofpacts_last->data, ofpacts_last->size); > } else { > - /* Split multicast groups with size greater than > - * MC_OFPACTS_MAX_MSG_SIZE in order to not overcome the > - * MAX_ACTIONS_BUFSIZE netlink buffer size supported by the kernel. > - * In order to avoid all the action buffers to be squashed together by > - * ovs, add a controller action for each configured openflow. > - */ > - size_t oc_offset = encode_start_controller_op( > - ACTION_OPCODE_MG_SPLIT_BUF, false, NX_CTLR_NO_METER, ofpacts); > - ovs_be32 val = htonl(++flow_index); > - ofpbuf_put(ofpacts, &val, sizeof val); > - val = htonl(mc->tunnel_key); > - ofpbuf_put(ofpacts, &val, sizeof val); > - ofpbuf_put(ofpacts, &stage, sizeof stage); > - encode_finish_controller_op(oc_offset, ofpacts); > + put_split_buf_function(++flow_index, mc->tunnel_key, stage, ofpacts); > } > > ofctrl_add_flow(flow_table, stage, prio, mc->header_.uuid.parts[0], > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 3fb7e2fd7..01a77bf93 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -209,7 +209,7 @@ static void send_mac_binding_buffered_pkts(struct rconn *swconn) > > static void pinctrl_rarp_activation_strategy_handler(const struct match *md); > > -static void pinctrl_mg_split_buff_handler( > +static void pinctrl_split_buf_action_handler( > struct rconn *swconn, struct dp_packet *pkt, > const struct match *md, struct ofpbuf *userdata); > > @@ -3772,9 +3772,9 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg) > ovs_mutex_unlock(&pinctrl_mutex); > break; > > - case ACTION_OPCODE_MG_SPLIT_BUF: > - pinctrl_mg_split_buff_handler(swconn, &packet, &pin.flow_metadata, > - &userdata); > + case ACTION_OPCODE_SPLIT_BUF_ACTION: > + pinctrl_split_buf_action_handler(swconn, &packet, &pin.flow_metadata, > + &userdata); > break; > > default: > @@ -8690,8 +8690,9 @@ pinctrl_rarp_activation_strategy_handler(const struct match *md) > } > > static void > -pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, > - const struct match *md, struct ofpbuf *userdata) > +pinctrl_split_buf_action_handler(struct rconn *swconn, struct dp_packet *pkt, > + const struct match *md, > + struct ofpbuf *userdata) > { > ovs_be32 *index = ofpbuf_try_pull(userdata, sizeof *index); > if (!index) { > @@ -8700,10 +8701,10 @@ pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, > return; > } > > - ovs_be32 *mg = ofpbuf_try_pull(userdata, sizeof *mg); > - if (!mg) { > + ovs_be32 *outport = ofpbuf_try_pull(userdata, sizeof *outport); > + if (!outport) { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > - VLOG_WARN_RL(&rl, "%s: missing multicast group field", __func__); > + VLOG_WARN_RL(&rl, "%s: missing outport tunnel_key field", __func__); > return; > } > > @@ -8725,7 +8726,7 @@ pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, > ofpact_put_set_field(&ofpacts, pkt_mark_field, &pkt_mark_value, NULL); > > put_load(ntohl(*index), MFF_REG6, 0, 32, &ofpacts); > - put_load(ntohl(*mg), MFF_LOG_OUTPORT, 0, 32, &ofpacts); > + put_load(ntohl(*outport), MFF_LOG_OUTPORT, 0, 32, &ofpacts); > > struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts); > resubmit->in_port = OFPP_CONTROLLER; > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index db7342f1d..7e0670a11 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -783,8 +783,8 @@ enum action_opcode { > /* activation_strategy_rarp() */ > ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, > > - /* multicast group split buffer action. */ > - ACTION_OPCODE_MG_SPLIT_BUF, > + /* split buffer action. */ > + ACTION_OPCODE_SPLIT_BUF_ACTION, > > /* "dhcp_relay_req_chk(relay_ip, server_ip)". > * > diff --git a/lib/actions.c b/lib/actions.c > index d5fc30b27..4a328b03d 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -1968,7 +1968,7 @@ is_paused_nested_action(enum action_opcode opcode) > case ACTION_OPCODE_HANDLE_SVC_CHECK: > case ACTION_OPCODE_BFD_MSG: > case ACTION_OPCODE_ACTIVATION_STRATEGY_RARP: > - case ACTION_OPCODE_MG_SPLIT_BUF: > + case ACTION_OPCODE_SPLIT_BUF_ACTION: > case ACTION_OPCODE_DHCP_RELAY_REQ_CHK: > case ACTION_OPCODE_DHCP_RELAY_RESP_CHK: > default: > -- > 2.47.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/physical.c b/controller/physical.c index adf37600d..1adc0a5f6 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -193,6 +193,27 @@ put_stack(enum mf_field_id field, struct ofpact_stack *stack) stack->subfield.n_bits = stack->subfield.field->n_bits; } +/* Split the ofpacts buffer to prevent overflow of the + * MAX_ACTIONS_BUFSIZE netlink buffer size supported by the kernel. + * In order to avoid all the action buffers to be squashed together by + * ovs, add a controller action for each configured openflow. + */ +static void +put_split_buf_function(uint32_t index, uint32_t outport, uint8_t stage, + struct ofpbuf *ofpacts) +{ + ovs_be32 values[2] = { + htonl(index), + htonl(outport) + }; + size_t oc_offset = + encode_start_controller_op(ACTION_OPCODE_SPLIT_BUF_ACTION, false, + NX_CTLR_NO_METER, ofpacts); + ofpbuf_put(ofpacts, values, sizeof values); + ofpbuf_put(ofpacts, &stage, sizeof stage); + encode_finish_controller_op(oc_offset, ofpacts); +} + static const struct sbrec_port_binding * get_localnet_port(const struct hmap *local_datapaths, int64_t tunnel_key) { @@ -2088,20 +2109,7 @@ mc_ofctrl_add_flow(const struct sbrec_multicast_group *mc, if (index == (mc->n_ports - 1)) { ofpbuf_put(ofpacts, ofpacts_last->data, ofpacts_last->size); } else { - /* Split multicast groups with size greater than - * MC_OFPACTS_MAX_MSG_SIZE in order to not overcome the - * MAX_ACTIONS_BUFSIZE netlink buffer size supported by the kernel. - * In order to avoid all the action buffers to be squashed together by - * ovs, add a controller action for each configured openflow. - */ - size_t oc_offset = encode_start_controller_op( - ACTION_OPCODE_MG_SPLIT_BUF, false, NX_CTLR_NO_METER, ofpacts); - ovs_be32 val = htonl(++flow_index); - ofpbuf_put(ofpacts, &val, sizeof val); - val = htonl(mc->tunnel_key); - ofpbuf_put(ofpacts, &val, sizeof val); - ofpbuf_put(ofpacts, &stage, sizeof stage); - encode_finish_controller_op(oc_offset, ofpacts); + put_split_buf_function(++flow_index, mc->tunnel_key, stage, ofpacts); } ofctrl_add_flow(flow_table, stage, prio, mc->header_.uuid.parts[0], diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 3fb7e2fd7..01a77bf93 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -209,7 +209,7 @@ static void send_mac_binding_buffered_pkts(struct rconn *swconn) static void pinctrl_rarp_activation_strategy_handler(const struct match *md); -static void pinctrl_mg_split_buff_handler( +static void pinctrl_split_buf_action_handler( struct rconn *swconn, struct dp_packet *pkt, const struct match *md, struct ofpbuf *userdata); @@ -3772,9 +3772,9 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg) ovs_mutex_unlock(&pinctrl_mutex); break; - case ACTION_OPCODE_MG_SPLIT_BUF: - pinctrl_mg_split_buff_handler(swconn, &packet, &pin.flow_metadata, - &userdata); + case ACTION_OPCODE_SPLIT_BUF_ACTION: + pinctrl_split_buf_action_handler(swconn, &packet, &pin.flow_metadata, + &userdata); break; default: @@ -8690,8 +8690,9 @@ pinctrl_rarp_activation_strategy_handler(const struct match *md) } static void -pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, - const struct match *md, struct ofpbuf *userdata) +pinctrl_split_buf_action_handler(struct rconn *swconn, struct dp_packet *pkt, + const struct match *md, + struct ofpbuf *userdata) { ovs_be32 *index = ofpbuf_try_pull(userdata, sizeof *index); if (!index) { @@ -8700,10 +8701,10 @@ pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, return; } - ovs_be32 *mg = ofpbuf_try_pull(userdata, sizeof *mg); - if (!mg) { + ovs_be32 *outport = ofpbuf_try_pull(userdata, sizeof *outport); + if (!outport) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - VLOG_WARN_RL(&rl, "%s: missing multicast group field", __func__); + VLOG_WARN_RL(&rl, "%s: missing outport tunnel_key field", __func__); return; } @@ -8725,7 +8726,7 @@ pinctrl_mg_split_buff_handler(struct rconn *swconn, struct dp_packet *pkt, ofpact_put_set_field(&ofpacts, pkt_mark_field, &pkt_mark_value, NULL); put_load(ntohl(*index), MFF_REG6, 0, 32, &ofpacts); - put_load(ntohl(*mg), MFF_LOG_OUTPORT, 0, 32, &ofpacts); + put_load(ntohl(*outport), MFF_LOG_OUTPORT, 0, 32, &ofpacts); struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts); resubmit->in_port = OFPP_CONTROLLER; diff --git a/include/ovn/actions.h b/include/ovn/actions.h index db7342f1d..7e0670a11 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -783,8 +783,8 @@ enum action_opcode { /* activation_strategy_rarp() */ ACTION_OPCODE_ACTIVATION_STRATEGY_RARP, - /* multicast group split buffer action. */ - ACTION_OPCODE_MG_SPLIT_BUF, + /* split buffer action. */ + ACTION_OPCODE_SPLIT_BUF_ACTION, /* "dhcp_relay_req_chk(relay_ip, server_ip)". * diff --git a/lib/actions.c b/lib/actions.c index d5fc30b27..4a328b03d 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -1968,7 +1968,7 @@ is_paused_nested_action(enum action_opcode opcode) case ACTION_OPCODE_HANDLE_SVC_CHECK: case ACTION_OPCODE_BFD_MSG: case ACTION_OPCODE_ACTIVATION_STRATEGY_RARP: - case ACTION_OPCODE_MG_SPLIT_BUF: + case ACTION_OPCODE_SPLIT_BUF_ACTION: case ACTION_OPCODE_DHCP_RELAY_REQ_CHK: case ACTION_OPCODE_DHCP_RELAY_RESP_CHK: default:
The action used for multiple split buffer scenario, when the action wouldn't fit into the netlink buffer, can be used in different context than just the multicast groups. Change the naming and add generic helper so it is clearer that is indeed the case. Signed-off-by: Ales Musil <amusil@redhat.com> --- controller/physical.c | 36 ++++++++++++++++++++++-------------- controller/pinctrl.c | 21 +++++++++++---------- include/ovn/actions.h | 4 ++-- lib/actions.c | 2 +- 4 files changed, 36 insertions(+), 27 deletions(-)