[ovs-dev] [PATCH 3/3] tunnel: set skb mark for IPsec tunnel packets

Pravin Shelar pshelar at nicira.com
Thu Feb 14 23:47:23 UTC 2013


On Thu, Feb 14, 2013 at 3:29 PM, Ansis Atteka <aatteka at nicira.com> wrote:
> On Thu, Feb 14, 2013 at 1:16 PM, Ben Pfaff <blp at nicira.com> wrote:
>> On Thu, Feb 14, 2013 at 01:13:29PM -0800, Pravin Shelar wrote:
>>> On Thu, Feb 14, 2013 at 11:50 AM, Ansis Atteka <aatteka at nicira.com> wrote:
>>> > The new ovs-monitor-ipsec implementation will use skb marks in
>>> > IPsec policies. This patch will configure datapath to use these
>>> > skb marks for IPsec tunnel packets.
>>> >
>>> > Issue: 14870
>>> > Signed-off-by: Ansis Atteka <aatteka at nicira.com>
>>> > ---
>>> >  lib/odp-util.c         |   12 +++++++++---
>>> >  lib/odp-util.h         |    4 ++--
>>> >  ofproto/ofproto-dpif.c |   11 +++++++----
>>> >  ofproto/tunnel.c       |    9 +++++++--
>>> >  ofproto/tunnel.h       |    5 ++++-
>>> >  5 files changed, 29 insertions(+), 12 deletions(-)
>>> >
>>> > diff --git a/lib/odp-util.c b/lib/odp-util.c
>>> > index 7e48981..755bdd6 100644
>>> > --- a/lib/odp-util.c
>>> > +++ b/lib/odp-util.c
>>> > @@ -2043,11 +2043,17 @@ odp_put_userspace_action(uint32_t pid, const union user_action_cookie *cookie,
>>> >
>>> >  void
>>> >  odp_put_tunnel_action(const struct flow_tnl *tunnel,
>>> > -                      struct ofpbuf *odp_actions)
>>> > +                      struct ofpbuf *odp_actions, uint32_t skb_mark)
>>> >  {
>>> >      size_t offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
>>> >      tun_key_to_attr(odp_actions, tunnel);
>>> >      nl_msg_end_nested(odp_actions, offset);
>>> > +
>>> > +    if (skb_mark) {
>>> > +        offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
>>> > +        nl_msg_put_u32(odp_actions, OVS_KEY_ATTR_SKB_MARK, skb_mark);
>>> > +        nl_msg_end_nested(odp_actions, offset);
>>> > +    }
>>> >  }
>>> >
>>> why not use one available in flow->skb_mark? So that existing set
>>> skb-mark action will take care of generating action for you.
>>
>> I agree, that's a better idea.
>>
>> (I personally forgot we had skb_mark in struct flow, maybe Ansis didn't
>> realize.)
>
> Perhaps I am misunderstanding something in the code, but I think that
> flow->skb_mark is still unset in send_packet() function after flow is
> extracted from the packet. Isn't it?
>
> That is the reason why I am using mark from ofport->tnl_port.
>

Yes, you shld extract skb_mark from tnl_port, but store it in
ctx->flow->skb_mark, rather than passing it as new param. that will
make sure odp actions will be created.
I think this will also take care of problem that Ben pointed out earlier.

> static int
> send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
> {
>     ...
>     flow_extract(packet, 0, 0, NULL, OFPP_LOCAL, &flow);
>     ...
>     if (ofport->tnl_port) {
>         ...
>         odp_port = tnl_port_send(ofport->tnl_port, &flow, &skb_mark);
>         ....
>         odp_put_tunnel_action(&flow.tunnel, &odp_actions, skb_mark);
>     } else {
>    ...
> }
>
> I guess this logic is relevant for packets that are composed in
> User-space (e.g. CFM)



More information about the dev mailing list