Hi Pravin,

Thanks for the feedback.
So another option is to add an new truncate action, and modify the size for
all the following output packets.  For example, the output:1 remains the
original size and the output:2 and output:3 will have size 100.
  # ovs-ofctl add-flow br0 'actions=output:1, truncate:100, output:2,
output:3'

Or we can implement truncate_output action so that the truncate only scope
at a specific output. For example:
  # ovs-ofctl add-flow br0 'actions=output:1, truncate_output:
(2,max_len=100), output:3'
So that only output:2 is re-sized to 100.

Which one do you think is more useful?

Regards,
William

On Tue, Mar 29, 2016 at 4:52 PM, pravin shelar <[email protected]> wrote:

> On Tue, Mar 29, 2016 at 3:16 PM, William Tu <[email protected]> wrote:
> > Before, OpenFlow specification defines 'max_len' in struct
> ofp_action_output
> > as the max number of bytes to send when port is OFPP_CONTROLLER.  A
> max_len
> > of zero means no bytes of the packet should be sent, and max_len of
> > OFPCML_NO_BUFFER means the complete packet is sent to the controller.
> > It is left undefined of max_len, when output port is not OFPP_CONTROLLER.
> > The patch extends the use of max_len when output is non-controller.
> >
> > One use case is to enable port mirroring to send smaller packets to the
> > destination port so that only useful packet information is
> mirrored/copied,
> > saving some performance overhead of copying entire packet payload.
> > The patch proposes adding a '(max_len=<N>)' after the output action.  An
> > example use case is below as well as shown in the tests/:
> >
> >     - Output to port 1 with max_len 100 bytes.
> >     - The output packet size on port 1 will be MIN(original_packet_size,
> 100).
> >     # ovs-ofctl add-flow br0 'actions=output:1(max_len=100)'
> >
> >     - The scope of max_len is limited to output action itself.  The
> following
> >       output:1 and output:2 will be the original packet size.
> >     # ovs-ofctl add-flow br0
> 'actions=output:1(max_len=100),output:1,output:2'
> >
> > Implementation/Limitaions:
> >     - Userspace and kernel datapath is supported, no Windows support.
> >     - The minimum value of max_len is 60 byte (minimum Ethernet frame
> size).
> >       This is defined in OVS_ACTION_OUTPUT_MIN.
> >     - Since 'max_len' is undefined in OpenFlow spec, the controller might
> >       accidentally place a garbage value in max_len and send to OVS.
> >     - For compatibility, if the kernel datapath is not supported, set
> >       max_len to zero.
> >     - OUTPUT_REG with max_len is not supported.
> >     - actions=1(max_len=100) is not supported, must specify as
> 'output:1'.
> >     - Only output:[0-9]*(max_len=<N>) is supported.  Output to IN_PORT,
> >       TABLE, NORMAL, FLOOD, ALL, and LOCAL are not supported.
> >
> > Signed-off-by: William Tu <[email protected]>
> > ---
> >  datapath/actions.c                                | 19 +++++--
> >  datapath/datapath.h                               |  1 +
> >  datapath/flow_netlink.c                           | 10 ++--
> >  datapath/linux/compat/include/linux/openvswitch.h |  7 +++
> >  datapath/vport.c                                  |  6 +++
> >  lib/dp-packet.c                                   |  1 +
> >  lib/dp-packet.h                                   |  1 +
> >  lib/dpctl.c                                       | 19 ++++---
> >  lib/dpif-netdev.c                                 | 20 ++++++-
> >  lib/netdev.c                                      |  8 +++
> >  lib/netlink.h                                     |  1 +
> >  lib/odp-util.c                                    | 27 ++++++++--
> >  lib/ofp-actions.c                                 | 41 +++++++++++++++
> >  lib/ofp-actions.h                                 |  4 +-
> >  ofproto/ofproto-dpif-xlate.c                      | 33 +++++++-----
> >  ofproto/ofproto-dpif.c                            | 45 ++++++++++++++++
> >  ofproto/ofproto-dpif.h                            |  4 ++
> >  tests/ofp-print.at                                |  6 +--
> >  tests/ofproto-dpif.at                             | 53
> +++++++++++++++++++
> >  tests/system-traffic.at                           | 63
> +++++++++++++++++++++++
> >  20 files changed, 330 insertions(+), 39 deletions(-)
> >
> > diff --git a/datapath/actions.c b/datapath/actions.c
> > index dcf8591..d64dadf 100644
> > --- a/datapath/actions.c
> > +++ b/datapath/actions.c
> > @@ -738,10 +738,15 @@ err:
> >  }
> >
> >  static void do_output(struct datapath *dp, struct sk_buff *skb, int
> out_port,
> > -                     struct sw_flow_key *key)
> > +                    uint16_t max_len, struct sw_flow_key *key)
> >  {
> >         struct vport *vport = ovs_vport_rcu(dp, out_port);
> >
> > +       /* This is after skb_clone called from do_execute_actions,
> > +          so max_len only applies to the current skb. */
> > +       if (unlikely(max_len != 0))
> > +               OVS_CB(skb)->max_len = max_len;
> > +
> >         if (likely(vport)) {
> >                 u16 mru = OVS_CB(skb)->mru;
> >
> > @@ -1034,6 +1039,7 @@ static int do_execute_actions(struct datapath *dp,
> struct sk_buff *skb,
> >          * is slightly obscure just to avoid that.
> >          */
> >         int prev_port = -1;
> > +       uint16_t max_len = 0;
> >         const struct nlattr *a;
> >         int rem;
> >
> > @@ -1045,15 +1051,18 @@ static int do_execute_actions(struct datapath
> *dp, struct sk_buff *skb,
> >                         struct sk_buff *out_skb = skb_clone(skb,
> GFP_ATOMIC);
> >
> >                         if (out_skb)
> > -                               do_output(dp, out_skb, prev_port, key);
> > +                               do_output(dp, out_skb, prev_port,
> max_len, key);
> >
> >                         prev_port = -1;
> >                 }
> >
> >                 switch (nla_type(a)) {
> > -               case OVS_ACTION_ATTR_OUTPUT:
> > -                       prev_port = nla_get_u32(a);
> > +               case OVS_ACTION_ATTR_OUTPUT: {
> > +                       struct ovs_action_output *output = nla_data(a);
> > +                       prev_port = output->port;
> > +                       max_len = output->max_len;
> >                         break;
> > +               }
>
> It is better to add separate action to truncate a packet rather than
> extending output action. Separate action would allow greater reuse of
> the action. For example we can use truncate action for sampling
> truncated packets.
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to