I had an offline discussion on this with Pravin. I wasn’t fully aware of the
kernel stack restrictions, and as this specific patch seemed to have little
impact, we’ll drop this for now.
Jarno
On Feb 12, 2014, at 3:48 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> 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
