| Message ID | 20230927001308.749910-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: openvswitch: Reduce stack usage | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 9/27/23 02:13, Nicholas Piggin wrote: > This takes do_execute_actions stack use from 544 bytes to 288 > bytes. execute_push_nsh uses 336 bytes, but it is a leaf call not > involved in recursion. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > net/openvswitch/actions.c | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) Hi, Nicholas. I made the same change about a week ago: https://lore.kernel.org/netdev/20230921194314.1976605-1-i.maximets@ovn.org/ So, you can drop this patch from your set. Best regards, Ilya Maximets. > > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c > index fd66014d8a76..8933caa92794 100644 > --- a/net/openvswitch/actions.c > +++ b/net/openvswitch/actions.c > @@ -1286,6 +1286,21 @@ static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key) > return 0; > } > > +static noinline_for_stack int execute_push_nsh(struct sk_buff *skb, > + struct sw_flow_key *key, > + const struct nlattr *attr) > +{ > + u8 buffer[NSH_HDR_MAX_LEN]; > + struct nshhdr *nh = (struct nshhdr *)buffer; > + int err; > + > + err = nsh_hdr_from_nlattr(attr, nh, NSH_HDR_MAX_LEN); > + if (likely(!err)) > + err = push_nsh(skb, key, nh); > + > + return err; > +} > + > /* Execute a list of actions against 'skb'. */ > static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, > struct sw_flow_key *key, > @@ -1439,17 +1454,9 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, > err = pop_eth(skb, key); > break; > > - case OVS_ACTION_ATTR_PUSH_NSH: { > - u8 buffer[NSH_HDR_MAX_LEN]; > - struct nshhdr *nh = (struct nshhdr *)buffer; > - > - err = nsh_hdr_from_nlattr(nla_data(a), nh, > - NSH_HDR_MAX_LEN); > - if (unlikely(err)) > - break; > - err = push_nsh(skb, key, nh); > + case OVS_ACTION_ATTR_PUSH_NSH: > + err = execute_push_nsh(skb, key, nla_data(a)); > break; > - } > > case OVS_ACTION_ATTR_POP_NSH: > err = pop_nsh(skb, key);
On Wed Sep 27, 2023 at 6:26 PM AEST, Ilya Maximets wrote: > On 9/27/23 02:13, Nicholas Piggin wrote: > > This takes do_execute_actions stack use from 544 bytes to 288 > > bytes. execute_push_nsh uses 336 bytes, but it is a leaf call not > > involved in recursion. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > net/openvswitch/actions.c | 27 +++++++++++++++++---------- > > 1 file changed, 17 insertions(+), 10 deletions(-) > > Hi, Nicholas. I made the same change about a week ago: > https://lore.kernel.org/netdev/20230921194314.1976605-1-i.maximets@ovn.org/ > So, you can drop this patch from your set. Ah nice, I didn't see it. Looks good to me. Thanks, Nick > > Best regards, Ilya Maximets. > > > > > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c > > index fd66014d8a76..8933caa92794 100644 > > --- a/net/openvswitch/actions.c > > +++ b/net/openvswitch/actions.c > > @@ -1286,6 +1286,21 @@ static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key) > > return 0; > > } > > > > +static noinline_for_stack int execute_push_nsh(struct sk_buff *skb, > > + struct sw_flow_key *key, > > + const struct nlattr *attr) > > +{ > > + u8 buffer[NSH_HDR_MAX_LEN]; > > + struct nshhdr *nh = (struct nshhdr *)buffer; > > + int err; > > + > > + err = nsh_hdr_from_nlattr(attr, nh, NSH_HDR_MAX_LEN); > > + if (likely(!err)) > > + err = push_nsh(skb, key, nh); > > + > > + return err; > > +} > > + > > /* Execute a list of actions against 'skb'. */ > > static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, > > struct sw_flow_key *key, > > @@ -1439,17 +1454,9 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, > > err = pop_eth(skb, key); > > break; > > > > - case OVS_ACTION_ATTR_PUSH_NSH: { > > - u8 buffer[NSH_HDR_MAX_LEN]; > > - struct nshhdr *nh = (struct nshhdr *)buffer; > > - > > - err = nsh_hdr_from_nlattr(nla_data(a), nh, > > - NSH_HDR_MAX_LEN); > > - if (unlikely(err)) > > - break; > > - err = push_nsh(skb, key, nh); > > + case OVS_ACTION_ATTR_PUSH_NSH: > > + err = execute_push_nsh(skb, key, nla_data(a)); > > break; > > - } > > > > case OVS_ACTION_ATTR_POP_NSH: > > err = pop_nsh(skb, key);
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c index fd66014d8a76..8933caa92794 100644 --- a/net/openvswitch/actions.c +++ b/net/openvswitch/actions.c @@ -1286,6 +1286,21 @@ static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key) return 0; } +static noinline_for_stack int execute_push_nsh(struct sk_buff *skb, + struct sw_flow_key *key, + const struct nlattr *attr) +{ + u8 buffer[NSH_HDR_MAX_LEN]; + struct nshhdr *nh = (struct nshhdr *)buffer; + int err; + + err = nsh_hdr_from_nlattr(attr, nh, NSH_HDR_MAX_LEN); + if (likely(!err)) + err = push_nsh(skb, key, nh); + + return err; +} + /* Execute a list of actions against 'skb'. */ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, struct sw_flow_key *key, @@ -1439,17 +1454,9 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb, err = pop_eth(skb, key); break; - case OVS_ACTION_ATTR_PUSH_NSH: { - u8 buffer[NSH_HDR_MAX_LEN]; - struct nshhdr *nh = (struct nshhdr *)buffer; - - err = nsh_hdr_from_nlattr(nla_data(a), nh, - NSH_HDR_MAX_LEN); - if (unlikely(err)) - break; - err = push_nsh(skb, key, nh); + case OVS_ACTION_ATTR_PUSH_NSH: + err = execute_push_nsh(skb, key, nla_data(a)); break; - } case OVS_ACTION_ATTR_POP_NSH: err = pop_nsh(skb, key);
This takes do_execute_actions stack use from 544 bytes to 288 bytes. execute_push_nsh uses 336 bytes, but it is a leaf call not involved in recursion. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- net/openvswitch/actions.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)