On Fri, Oct 21, 2011 at 4:24 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Thu, Oct 20, 2011 at 05:11:33PM -0700, Jesse Gross wrote:
>> On Mon, Oct 17, 2011 at 3:14 PM, Ben Pfaff <b...@nicira.com> wrote:
>> > diff --git a/datapath/tunnel.c b/datapath/tunnel.c
>> > index 8edff06..6fde389 100644
>> > --- a/datapath/tunnel.c
>> > +++ b/datapath/tunnel.c
>> > int tnl_set_addr(struct vport *vport, const unsigned char *addr)
>> > {
>> > struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
>> > - struct tnl_mutable_config *mutable;
>> > + struct tnl_mutable_config *old_mutable, *mutable;
>> >
>> > - mutable = kmemdup(rtnl_dereference(tnl_vport->mutable),
>> > - sizeof(struct tnl_mutable_config), GFP_KERNEL);
>> > + old_mutable = rtnl_dereference(tnl_vport->mutable);
>> > + mutable = kmemdup(old_mutable, sizeof(struct tnl_mutable_config),
>> > GFP_KERNEL);
>> > if (!mutable)
>> > return -ENOMEM;
>> >
>> > + mutable->mlink = mutable->mlink;
>>
>> I think this line probably isn't going to do a whole lot...
>
> Oops. The one part that I don't test...
>
> Changed to:
> mutable->mlink = old_mutable->mlink;
Actually, I think this line is completely unnecessary because it
follows kmemdup().
>> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> > index d579b87..d09ade4 100644
>> > --- a/vswitchd/vswitch.xml
>> > +++ b/vswitchd/vswitch.xml
>> > <column name="options" key="local_ip">
>> > - Optional. The destination IP that received packets must
>> > - match. Default is to match all addresses.
>> > + Optional. The destination IP that received packets must match.
>> > + Default is to match all addresses. Must be omitted when <ref
>> > + column="options" key="remote_ip"/> is a multicast address.
>> > </column>
>>
>> I think we probably should do some validation on this - otherwise
>> ports that specify a local address and a multicast address silently
>> won't match.
>
> OK, how's this:
Looks good.
>> Maybe we should also document that routing table changes for multicast
>> tunnels require deleting and recreating them.
>
> It's an embarrassing limitation. For now:
OK, that's fine.
>> I think in the gre_err() code path, we should ignore ICMP messages
>> sent to multicast addresses.
>
> OK, like this?
Yes, looks good as well.
So other than that one line looks good:
Acked-by: Jesse Gross <je...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
