On Tue, Feb 11, 2014 at 4:07 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
> ---
> datapath/datapath.c | 60
> +++++++++++++++++++++++++++------------------------
> 1 file changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index a46ceb0..90fdc60 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -492,14 +492,24 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
> struct genl_info *info)
> {
> struct ovs_header *ovs_header = info->userhdr;
> struct nlattr **a = info->attrs;
> - struct sw_flow_actions *acts;
> struct sk_buff *packet;
> - struct sw_flow *flow;
> struct datapath *dp;
> struct ethhdr *eth;
> + struct sw_flow flow;
> + struct {
> + struct sw_flow_actions acts;
> + u64 buf[512 / sizeof (u64)];
> + } sw_acts;
> + struct sw_flow_actions *acts;
> int len;
> int err;
>
I am not sure about stack usage here. Specifically on platforms with
4KB stack we can overrun kernel stack.
> + /* Start with actions in the stack with enough space for most cases.
> */
> + acts = &sw_acts.acts;
> + acts->actions_len = 0;
> + acts->alloc_size = sizeof(sw_acts);
> + acts->alloced = false;
> +
> err = -EINVAL;
> if (!a[OVS_PACKET_ATTR_PACKET] || !a[OVS_PACKET_ATTR_KEY] ||
> !a[OVS_PACKET_ATTR_ACTIONS])
> @@ -525,54 +535,48 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
> struct genl_info *info)
> else
> packet->protocol = htons(ETH_P_802_2);
>
> - /* Build an sw_flow for sending this packet. */
> - flow = ovs_flow_alloc();
> - err = PTR_ERR(flow);
> - if (IS_ERR(flow))
> - goto err_kfree_skb;
> -
> - err = ovs_flow_extract(packet, -1, &flow->key);
> + /* Build an sw_flow for sending this packet.
> + * We do not initialize mask, nor stats as they are not used
> + * by a flow that is not in the flow table. */
> + err = ovs_flow_extract(packet, -1, &flow.key);
> if (err)
> - goto err_flow_free;
> + goto err_free;
>
> - err = ovs_nla_get_flow_metadata(flow, a[OVS_PACKET_ATTR_KEY]);
> + err = ovs_nla_get_flow_metadata(&flow, a[OVS_PACKET_ATTR_KEY]);
> if (err)
> - goto err_flow_free;
> - acts =
> ovs_nla_alloc_flow_actions(nla_len(a[OVS_PACKET_ATTR_ACTIONS]));
> - err = PTR_ERR(acts);
> - if (IS_ERR(acts))
> - goto err_flow_free;
> + goto err_free;
>
> err = ovs_nla_copy_actions(a[OVS_PACKET_ATTR_ACTIONS],
> - &flow->key, 0, &acts);
> - rcu_assign_pointer(flow->sf_acts, acts);
> + &flow.key, 0, &acts);
> + rcu_assign_pointer(flow.sf_acts, acts);
> if (err)
> - goto err_flow_free;
> + goto err_free_acts;
>
> - OVS_CB(packet)->flow = flow;
> - OVS_CB(packet)->pkt_key = &flow->key;
> - packet->priority = flow->key.phy.priority;
> - packet->mark = flow->key.phy.skb_mark;
> + OVS_CB(packet)->flow = &flow;
> + OVS_CB(packet)->pkt_key = &flow.key;
> + packet->priority = flow.key.phy.priority;
> + packet->mark = flow.key.phy.skb_mark;
>
> rcu_read_lock();
> dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
> err = -ENODEV;
> if (!dp)
> goto err_unlock;
> -
> local_bh_disable();
> err = ovs_execute_actions(dp, packet);
> local_bh_enable();
> rcu_read_unlock();
>
> - ovs_flow_free(flow, false);
> + /* Will free if actions were realloced. */
> + ovs_nla_free_flow_actions(acts, false);
> +
> return err;
>
> err_unlock:
> rcu_read_unlock();
> -err_flow_free:
> - ovs_flow_free(flow, false);
> -err_kfree_skb:
> +err_free_acts:
> + ovs_nla_free_flow_actions(acts, false);
> +err_free:
> kfree_skb(packet);
> err:
> return err;
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
