On Tue, Nov 11, 2014 at 2:48 AM, Thomas Graf <tg...@noironetworks.com> wrote:
> On 11/10/14 at 02:52pm, Andy Zhou wrote:
>> Implements ip_defrag action in Linux kenrel using ip_defrag kernel APIs.
>>
>> Signed-off-by: Andy Zhou <az...@nicira.com>
>> ---
>> datapath/actions.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> datapath/flow_netlink.c | 6 +++++-
>> 2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index 5a1dbe2..668d44f 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>> @@ -826,6 +826,38 @@ static int execute_recirc(struct datapath *dp, struct
>> sk_buff *skb,
>> return 0;
>> }
>>
>> +static int ipv4_gather_frags(struct sk_buff *skb, u_int32_t user)
>> +{
>> + int err;
>
> Don't you need to pull off the ethernet header first? ip_defrag()
> expects an IP header at skb->data.
I could be wrong on this, but It looks to me ip_defrag uses ip_hdr()
to get the IP header information, thus pull
off ethernet header is not necessary.
>
>> +
>> + local_bh_disable();
>> + err = ip_defrag(skb, user);
>> + local_bh_enable();
>> +
>> + if (!err) {
>> + ip_send_check(ip_hdr(skb));
>> + skb->ignore_df = 1;
>> + }
>> +
>> + return err;
>> +}
>> +
>> +static int execute_ip_defrag(struct sk_buff *skb, struct sw_flow_key *key,
>> + const struct ovs_action_ip_defrag *act_ip_defrag)
>> +{
>> + if (key->eth.type == htons(ETH_P_IP)) {
>> + if (ip_is_fragment(ip_hdr(skb))) {
>> + enum ip_defrag_users user;
>> +
>> + user = IP_DEFRAG_CONNTRACK_IN + act_ip_defrag->zone;
>> + if (ipv4_gather_frags(skb, user))
>> + return -EINPROGRESS;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>
> Given I remember the defrag implementation correctly:
>
> We have to be clear about the side effects of this if we are reusing the
> existing CONNTRACK_IN zones. A packet processed in parallel outside of
> OVS on another net_device in the same net namespace can be reassembled
> into the OVS owned skb even though that packet may not match the original
> flow. The iif check not part of the ipq lookup.
Should zone be assigned in such a way to avoid those conflicts? In
addition, OVS IP_DEFRAG
can skip any SKBs that has already filtered by netfilter.
A cleaner option is to add new zone for OVS so we can avoid reusing
CONTRACK_IN zone.
Technically this cleaner. OVS IP_DEFRAG can be applied to either IN or
OUT direction,
depending on OF rules, thus only one zone is needed. I did not add a
new zone for this patch
since I have not found a good way to add compat header for it.
>
> A typical example would be:
>
> in_port(2),actions=ip_defrag(),...
Why is this typical?
>
> I think this is fine but needs to be documented.
Sure.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
