datapath: simplify sample action implementation
authorAndy Zhou <azhou@nicira.com>
Wed, 4 Mar 2015 11:26:06 +0000 (03:26 -0800)
committerPravin B Shelar <pshelar@nicira.com>
Wed, 4 Mar 2015 22:52:41 +0000 (14:52 -0800)
commitb953042214201e2693a485a8ba8b19f69e5bdf34
tree780bbcedec93e15c160b4e000e6e7fd452b792fa
parent5052f72ec9a0377929f41a4822c2a438012f7d06
datapath: simplify sample action implementation

The current sample() function implementation is more complicated
than necessary in handling single user space action optimization
and skb reference counting. There is no functional changes.

Msg from Chris Dunlop:

The commit isn't actually designed to address the problem directly,
however in simplifying sample() it removes a call to skb_get(). The
skb_get() makes the skb shared, which later causes us to hit the BUG().
E.g. your v2.3.1 stack trace shows this call path:

  netdev_frame_hook
  + netdev_port_receive
    | skb is guaranteed not-shared, via:
    |   skb = skb_share_check(skb, GFP_ATOMIC);
    + ovs_vport_receive
      + ovs_dp_process_received_packet
        + ovs_dp_process_packet_with_key
          + ovs_execute_actions
            + do_execute_actions
              | nla_type(a) == OVS_ACTION_ATTR_SAMPLE
              + sample
                | skb is made shared here, via:
                |   sample_skb = skb;
                |   skb_get(skb);
                + do_execute_actions
                  | nla_type(a) == OVS_ACTION_ATTR_USERSPACE
                  + output_userspace
                    + ovs_dp_upcall
                      + queue_userspace_packet
                        + skb_checksum_help
                          + pskb_expand_head
                            | if (skb_shared(skb))
                            |         BUG();        BOOM!!!

Reported-by: Chris Dunlop <chris@onthe.net.au>
Reported-by: "Xu (Simon) Chen" <xchenum@gmail.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
datapath/actions.c