On Tue, Jun 16, 2015 at 07:48:58PM -0300, Flavio Leitner wrote:
> On Tue, Jun 16, 2015 at 07:09:25PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > On Tue, Jun 16, 2015 at 06:25:47PM -0300, Flavio Leitner wrote:
> > > On Tue, Jun 16, 2015 at 06:01:09PM -0300, Thadeu Lima de Souza Cascardo
> > > wrote:
> > > > IGMP packets need to take the slow path. Otherwise, packets that match
> > > > the same flow will not be processed by OVS. That might prevent OVS from
> > > > updating the expire time for entries already in the mdb, but also to
> > > > lose packets with different addresses in the payload.
> > > >
> > > > Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@redhat.com>
> > > > ---
> > > > ofproto/ofproto-dpif-xlate.c | 14 ++++++++++----
> > > > 1 file changed, 10 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > > > index f5dc272..6fcf7b8 100644
> > > > --- a/ofproto/ofproto-dpif-xlate.c
> > > > +++ b/ofproto/ofproto-dpif-xlate.c
> > > > @@ -2269,12 +2269,18 @@ xlate_normal(struct xlate_ctx *ctx)
> > > > struct mcast_group *grp;
> > > >
> > > > if (flow->nw_proto == IPPROTO_IGMP) {
> > > > - if (ctx->xin->may_learn) {
> > > > - if (mcast_snooping_is_membership(flow->tp_src) ||
> > > > - mcast_snooping_is_query(flow->tp_src)) {
> > > > + if (mcast_snooping_is_membership(flow->tp_src) ||
> > > > + mcast_snooping_is_query(flow->tp_src)) {
> > > > + if (ctx->xin->may_learn) {
> > > > update_mcast_snooping_table(ctx->xbridge, flow,
> > > > vlan,
> > > > in_xbundle);
> > > > - }
> > > > + }
> > > > + /*
> > > > + * IGMP packets need to take the slow path, in order
> > > > to be
> > > > + * processed for mdb updates. That will prevent expires
> > > > + * firing off even after hosts have sent reports.
> > > > + */
> > >
> > > looks like the above isn't correct aligned.
> > >
> > > I forgot to tell before about updating the NEWS file:
> > >
> > > - Support for multicast snooping (IGMPv1 and IGMPv2
> > >
> > > and to update vswitchd/vswitch.xml:
> > > <group title="Multicast Snooping Configuration">
> > > Multicast snooping (RFC 4541) monitors the Internet Group Management
> > > Protocol (IGMP) traffic between hosts and multicast routers. The
> > > switch uses what IGMP snooping learns to forward multicast traffic
> > > only to interfaces that are connected to interested receivers.
> > > Currently it supports IGMPv1 and IGMPv2 protocols.
> > >
> > > fbl
> > >
> >
> > Thanks a lot for the review, Flavio.
> >
> > Excuse my lack of attention to the details. I miss something like
> > checkpatch on Linux, that would take care of some of the coding style
> > problems one may find in a patch. Is there any recommendations on that
> > for OVS?
>
> A checkpatch script for OVS would be very welcome.
>
> > I have a v5 prepared, but will wait for any other comments before I send
> > it.
>
> Sounds good to me.
> Perhaps Ben has something else to add.
Seems like a thorough review to me, I'll expect to apply v5.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
