Re: [sfc-dev] [netvirt-dev] Networking SFC convertor in ODL

2016-10-04 Thread Sam Hague
On Tue, Oct 4, 2016 at 5:38 PM, Anil Vishnoi  wrote:

> Hi Brady,
>
> Please see inline..
>
> On Tue, Oct 4, 2016 at 12:47 PM, Brady Johnson <
> bradyallenjohn...@gmail.com> wrote:
>
>>
>> Anil,
>>
>> We're talking about 2 different patches here.
>>
>> I remember patch [0] well, and the discussions around it, which you
>> quoted above. Im not that old yet ;) This patch is the Neutron Northbound
>> API.
>>
>> I dont remember ever hearing anything about patch [1] though. This patch
>> is the *implementation* of the Northbound API from patch [0]. I was
>> under the impression that the implementation was going to be put in ODL SFC.
>>
>
> ​Not sure how you got this impression, given that the only response we got
> was pretty strong against this idea :-).
>
Agreed, though I recall something along the lines of "as long as the odl
sfc models are left alone do whatever". We didn't have any other feedback
and Anil needed to make progress so we did what we could to move forward.

>
> "
> >* This ISN’T you exercising the right to do your own implementation of
> Neutron SFC, this is introducing an unwanted API dependency on ODL-SFC.*
>
> >>* So now if ODL SFC needs/wants to change its YANG models due to say IETF / 
> >>sanity, and those don’t magically line up with neutron-sfc’s bizarre view 
> >>of the world, or at least one beyond 1999-2001 when this sort of approach 
> >>to SFC was first proposed, we get blocked from doing so because it will 
> >>break this “translation layer” that now has welded an unwanted dependency 
> >>on the project. *
>
> >"​
>
>
>> Considering the patch is experimental as you mentioned, I can see why you
>> didnt publish it.
>>
>
> ​This patch was out there in the open for everyone to review in NetVirt
> master branch, so anyone can review these patch. Going forward we will add
> all the sfc committers as a reviewer in the NetVirt patches that is related
> to SFC work, so that they can easily monitor those patches. Hopefully that
> won't create much spam for them.
>
Agreed here also. The patches sat around for a while and were in the open.
It was an oversight that more reviewers were not included - that is
something we need to be better about.

>
>
>> Can you please mark experimental patches as such in the future to avoid
>> confusion.
>>
>
> ​Yup, i think that's useful to have.​
>
>
>> Also, It wouldnt hurt to mention why its being self-merged.
>>
> ​Agree, what standard practice SFC project use?, we can probably follow
> the same in net-virt project ?​
>
To be fair, I reviewed the patch but didn't have substantial comments. Anil
and I talked about it and I said it was OK to merge.

>
>
> These additional comments would have probably saved some work for Alexis
>> too.
>>
> ​I think alexis comments were very valid comments, so i think his time was
> well spent :).
>
>>
>> As you can imagine, the ODL SFC community is very interested to see how
>> the implementation of patch [0] works out. I was hoping it would be
>> possible to wait until Networking SFC aligned with either IETF SFC or ETSI
>> VNFFG, else we risk not being able to completely leverage ODL SFC, since
>> the Networking SFC API is very basic and lacking to say the least.
>>
> ​I think it's a longer discussion that need to be done across
> openstack/opendaylight/opnfv. I am not sure what are the concrete steps are
> taken by networking-sfc project to move toward this direction. I saw one
> blueprint in this direction, but it was abandoned later on. I think there
> is a long way ahead to get to those final api we are expecting, and i think
> this requires some effort from teh provider side to implement the available
> API's and provide the feedback to the networking-sfc. I think that way we
> can make faster progress.
>
As you know the APIs have been taking some time to settle, so we moved
ahead with what we had at the time and made sure not to impact odl sfc.
That seems to be what you are concerned with so I think we met that need.

>
>> In the future, could you please inform us of any additional work in this
>> area so we can help and advise if needed.
>>
> ​Sure will do, please keep watching the sfc-dev mailing list ;).​
>
>
>>
>> Thanks,
>>
>> Brady
>>
>> [0] https://git.opendaylight.org/gerrit/#/c/38748/
>> [1] https://git.opendaylight.org/gerrit/#/c/43305/
>>
>>
>> On Tue, Oct 4, 2016 at 8:05 PM, Anil Vishnoi 
>> wrote:
>>
>>> Hi Brady,
>>>
>>> Sorry to say, but most of the questions that you asked below is really a
>>> surprise to me as well, given that you were well aware of the overall
>>> networking-sfc integration work and translation layer work that i was doing
>>> and *it was discussed on sfc-dev mailing list. *Even you were reviewer
>>> of the first ever patch that I pushed in ODL to start this work.
>>>
>>> https://git.opendaylight.org/gerrit/#/c/38748/
>>>
>>> Please see inline...
>>>
>>> On Tue, Oct 4, 2016 at 12:30 AM, Brady Allen Johnson <
>>> brady.allen.john...@ericsson.com> wrote:
>>>

 I just noticed 

Re: [sfc-dev] Networking SFC convertor in ODL

2016-10-04 Thread Anil Vishnoi
Hi Brady,

Please see inline..

On Tue, Oct 4, 2016 at 12:47 PM, Brady Johnson 
wrote:

>
> Anil,
>
> We're talking about 2 different patches here.
>
> I remember patch [0] well, and the discussions around it, which you quoted
> above. Im not that old yet ;) This patch is the Neutron Northbound API.
>
> I dont remember ever hearing anything about patch [1] though. This patch
> is the *implementation* of the Northbound API from patch [0]. I was under
> the impression that the implementation was going to be put in ODL SFC.
>

​Not sure how you got this impression, given that the only response we got
was pretty strong against this idea :-).

"
>* This ISN’T you exercising the right to do your own implementation of
Neutron SFC, this is introducing an unwanted API dependency on ODL-SFC.*

>>* So now if ODL SFC needs/wants to change its YANG models due to say IETF / 
>>sanity, and those don’t magically line up with neutron-sfc’s bizarre view of 
>>the world, or at least one beyond 1999-2001 when this sort of approach to SFC 
>>was first proposed, we get blocked from doing so because it will break this 
>>“translation layer” that now has welded an unwanted dependency on the 
>>project. *

>"​


> Considering the patch is experimental as you mentioned, I can see why you
> didnt publish it.
>

​This patch was out there in the open for everyone to review in NetVirt
master branch, so anyone can review these patch. Going forward we will add
all the sfc committers as a reviewer in the NetVirt patches that is related
to SFC work, so that they can easily monitor those patches. Hopefully that
won't create much spam for them.


> Can you please mark experimental patches as such in the future to avoid
> confusion.
>

​Yup, i think that's useful to have.​


> Also, It wouldnt hurt to mention why its being self-merged.
>
​Agree, what standard practice SFC project use?, we can probably follow the
same in net-virt project ?​


These additional comments would have probably saved some work for Alexis
> too.
>
​I think alexis comments were very valid comments, so i think his time was
well spent :).

>
> As you can imagine, the ODL SFC community is very interested to see how
> the implementation of patch [0] works out. I was hoping it would be
> possible to wait until Networking SFC aligned with either IETF SFC or ETSI
> VNFFG, else we risk not being able to completely leverage ODL SFC, since
> the Networking SFC API is very basic and lacking to say the least.
>
​I think it's a longer discussion that need to be done across
openstack/opendaylight/opnfv. I am not sure what are the concrete steps are
taken by networking-sfc project to move toward this direction. I saw one
blueprint in this direction, but it was abandoned later on. I think there
is a long way ahead to get to those final api we are expecting, and i think
this requires some effort from teh provider side to implement the available
API's and provide the feedback to the networking-sfc. I think that way we
can make faster progress.

>
> In the future, could you please inform us of any additional work in this
> area so we can help and advise if needed.
>
​Sure will do, please keep watching the sfc-dev mailing list ;).​


>
> Thanks,
>
> Brady
>
> [0] https://git.opendaylight.org/gerrit/#/c/38748/
> [1] https://git.opendaylight.org/gerrit/#/c/43305/
>
>
> On Tue, Oct 4, 2016 at 8:05 PM, Anil Vishnoi 
> wrote:
>
>> Hi Brady,
>>
>> Sorry to say, but most of the questions that you asked below is really a
>> surprise to me as well, given that you were well aware of the overall
>> networking-sfc integration work and translation layer work that i was doing
>> and *it was discussed on sfc-dev mailing list. *Even you were reviewer
>> of the first ever patch that I pushed in ODL to start this work.
>>
>> https://git.opendaylight.org/gerrit/#/c/38748/
>>
>> Please see inline...
>>
>> On Tue, Oct 4, 2016 at 12:30 AM, Brady Allen Johnson <
>> brady.allen.john...@ericsson.com> wrote:
>>
>>>
>>> I just noticed a *merged* Gerrit patch in Netvirt [0] to do the
>>> Networking SFC to ODL SFC conversion.
>>>
>>> I have to say that I was quite surprised to see that this patch was
>>> merged without ever consulting the ODL SFC community. I have a few
>>> questions regarding this patch:
>>>
>>>
>>>1. Why is this code in Netvirt and not in SFC where it belongs?
>>>
>>> Following thread ​actually discuss about why we implemented this
>> translation layer it in netvirt and not in sfc project. To explictly
>> mention here, I came to SFC project with the intent of implementing this
>> layer in ODL SFC project as one of your response says in the mail thread
>>
>> *"As for implementations of the "translation layer", I think Anil *
>>
>> *envisioned this to live in ODL SFC, and in any other projects that
>> wanted to implement their own form of SFC.
>>
>> Regards,
>>
>> Brady​"*
>>
>> ​Thread:*https://lists.opendaylight.org/pipermail/sfc-dev/2016-May/002998.html
>>  

Re: [sfc-dev] Networking SFC convertor in ODL

2016-10-04 Thread Brady Johnson
Anil,

We're talking about 2 different patches here.

I remember patch [0] well, and the discussions around it, which you quoted
above. Im not that old yet ;) This patch is the Neutron Northbound API.

I dont remember ever hearing anything about patch [1] though. This patch is
the *implementation* of the Northbound API from patch [0]. I was under the
impression that the implementation was going to be put in ODL SFC.
Considering the patch is experimental as you mentioned, I can see why you
didnt publish it. Can you please mark experimental patches as such in the
future to avoid confusion. Also, It wouldnt hurt to mention why its being
self-merged. These additional comments would have probably saved some work
for Alexis too.

As you can imagine, the ODL SFC community is very interested to see how the
implementation of patch [0] works out. I was hoping it would be possible to
wait until Networking SFC aligned with either IETF SFC or ETSI VNFFG, else
we risk not being able to completely leverage ODL SFC, since the Networking
SFC API is very basic and lacking to say the least.

In the future, could you please inform us of any additional work in this
area so we can help and advise if needed.

Thanks,

Brady

[0] https://git.opendaylight.org/gerrit/#/c/38748/
[1] https://git.opendaylight.org/gerrit/#/c/43305/


On Tue, Oct 4, 2016 at 8:05 PM, Anil Vishnoi  wrote:

> Hi Brady,
>
> Sorry to say, but most of the questions that you asked below is really a
> surprise to me as well, given that you were well aware of the overall
> networking-sfc integration work and translation layer work that i was doing
> and *it was discussed on sfc-dev mailing list. *Even you were reviewer of
> the first ever patch that I pushed in ODL to start this work.
>
> https://git.opendaylight.org/gerrit/#/c/38748/
>
> Please see inline...
>
> On Tue, Oct 4, 2016 at 12:30 AM, Brady Allen Johnson <
> brady.allen.john...@ericsson.com> wrote:
>
>>
>> I just noticed a *merged* Gerrit patch in Netvirt [0] to do the
>> Networking SFC to ODL SFC conversion.
>>
>> I have to say that I was quite surprised to see that this patch was
>> merged without ever consulting the ODL SFC community. I have a few
>> questions regarding this patch:
>>
>>
>>1. Why is this code in Netvirt and not in SFC where it belongs?
>>
>> Following thread ​actually discuss about why we implemented this
> translation layer it in netvirt and not in sfc project. To explictly
> mention here, I came to SFC project with the intent of implementing this
> layer in ODL SFC project as one of your response says in the mail thread
>
> *"As for implementations of the "translation layer", I think Anil *
>
> *envisioned this to live in ODL SFC, and in any other projects that
> wanted to implement their own form of SFC.
>
> Regards,
>
> Brady​"*
>
> ​Thread:*https://lists.opendaylight.org/pipermail/sfc-dev/2016-May/002998.html
>  *​
>
> If you read through this mail thread, keith was the only sfc committer who 
> provided his feedback and he was not in favor of implementing this layer in 
> the sfc project. We did not get any vote/opinion from you on this subject, 
> and the only feedback we got was not in favor of the idea, so that's the 
> reason we decided to implement it in netvirt and you were very well aware of 
> that as well.
>
> ​
>
>
>>1. Why wasnt the SFC community ever informed about these changes?
>>
>> ​I think this is not correct, I notified opendaylight sfc community even
> when I pushed blueprint for networking-sfc driver to openstack
> networking-sfc project. Here is the mail thread
>
> https://lists.opendaylight.org/pipermail/sfc-dev/2016-April/002833.html
>
> Apart from that you were reviewer on the gerrit i mentioned above, and
> that was the first patch on the ODL side that I pushed for networking-sfc
> support.
>
>>
>>1.
>>2. Why was the patch +2'd and merged by the author? Unless things
>>have changed drastically lately, self-merging is frowned upon, and in most
>>projects its simply *not allowed*.
>>
>> ​This is the first experimental version that we added for networking-sfc
> API support. Nature of this work was a experimental moreover to analyze the
> gap between networking-sfc and odl SFC API's and provide more feedback to
> networking-sfc project to improve their APIs. And Sam and I agreed on this
> that we can self merge this patch, given that it's totally isolated piece
> of work and don't interfere with netvirt project's functionality or ODL SFC
> project. Apart from that, sam did review initial patches with me before i
> merge these patches.
>
>>
>>1. There are some code review comments on the patch that were never
>>addressed, will these ever be addressed?
>>
>> ​As alexis mentioned in his comment below, he posted this comment
> recently (Sep 17)​
>
> ​and patches were merged on Aug 8 (before M5). I generally don't ​revisit
> my merged patches for comments, an

Re: [sfc-dev] Networking SFC convertor in ODL

2016-10-04 Thread Anil Vishnoi
Hi Brady,

Sorry to say, but most of the questions that you asked below is really a
surprise to me as well, given that you were well aware of the overall
networking-sfc integration work and translation layer work that i was doing
and *it was discussed on sfc-dev mailing list. *Even you were reviewer of
the first ever patch that I pushed in ODL to start this work.

https://git.opendaylight.org/gerrit/#/c/38748/

Please see inline...

On Tue, Oct 4, 2016 at 12:30 AM, Brady Allen Johnson <
brady.allen.john...@ericsson.com> wrote:

>
> I just noticed a *merged* Gerrit patch in Netvirt [0] to do the
> Networking SFC to ODL SFC conversion.
>
> I have to say that I was quite surprised to see that this patch was merged
> without ever consulting the ODL SFC community. I have a few questions
> regarding this patch:
>
>
>1. Why is this code in Netvirt and not in SFC where it belongs?
>
> Following thread ​actually discuss about why we implemented this
translation layer it in netvirt and not in sfc project. To explictly
mention here, I came to SFC project with the intent of implementing this
layer in ODL SFC project as one of your response says in the mail thread

*"As for implementations of the "translation layer", I think Anil *

*envisioned this to live in ODL SFC, and in any other projects that
wanted to implement their own form of SFC.

Regards,

Brady​"*

​Thread:*https://lists.opendaylight.org/pipermail/sfc-dev/2016-May/002998.html
*​

If you read through this mail thread, keith was the only sfc committer
who provided his feedback and he was not in favor of implementing this
layer in the sfc project. We did not get any vote/opinion from you on
this subject, and the only feedback we got was not in favor of the
idea, so that's the reason we decided to implement it in netvirt and
you were very well aware of that as well.

​


>1. Why wasnt the SFC community ever informed about these changes?
>
> ​I think this is not correct, I notified opendaylight sfc community even
when I pushed blueprint for networking-sfc driver to openstack
networking-sfc project. Here is the mail thread

https://lists.opendaylight.org/pipermail/sfc-dev/2016-April/002833.html

Apart from that you were reviewer on the gerrit i mentioned above, and that
was the first patch on the ODL side that I pushed for networking-sfc
support.

>
>1.
>2. Why was the patch +2'd and merged by the author? Unless things have
>changed drastically lately, self-merging is frowned upon, and in most
>projects its simply *not allowed*.
>
> ​This is the first experimental version that we added for networking-sfc
API support. Nature of this work was a experimental moreover to analyze the
gap between networking-sfc and odl SFC API's and provide more feedback to
networking-sfc project to improve their APIs. And Sam and I agreed on this
that we can self merge this patch, given that it's totally isolated piece
of work and don't interfere with netvirt project's functionality or ODL SFC
project. Apart from that, sam did review initial patches with me before i
merge these patches.

>
>1. There are some code review comments on the patch that were never
>addressed, will these ever be addressed?
>
> ​As alexis mentioned in his comment below, he posted this comment recently
(Sep 17)​

​and patches were merged on Aug 8 (before M5). I generally don't ​revisit
my merged patches for comments, and likely most of the developers too, and
that's the reason i probably have missed it. Alexis comment is already
partially addressed in other patches.

>
>1.
>
>
> Needless to say, this has left a nasty taste in my mouth. Hopefully we can
> set this straight and start collaborating between projects in the future.
>
Definitely, ​That was my primary intend, but unfortunately we didn't get
much enthusiastic ​response from sfc committers. Looking forward to a
better collaboration in future.
I am assuming that you are totally swamped with the mails and probably
missed some of the mail thread i mentioned above, let me know if i can
communicate with you by some other means that works better for you.

>
> Regards,
>
> Brady
>
> [0] https://git.opendaylight.org/gerrit/#/c/43305/
>
>


-- 
Thanks
Anil
___
sfc-dev mailing list
sfc-dev@lists.opendaylight.org
https://lists.opendaylight.org/mailman/listinfo/sfc-dev


Re: [sfc-dev] Networking SFC convertor in ODL

2016-10-04 Thread Alexis de Talhouët
Brady,

I can’t really say much beside the fact I had to fix an issue, this is why I 
got to see this and leave comments on the patch, but it was already merged.
I can’t give any insight regarding the motivation behind this SFC translator 
and why it resides in Netvirt rather than in SFC.

Anil, as the initial author, can you provide insight about the SFC Translator 
bundle? Maybe it could be moved to SFC project, as its own renderer.
Also, it seems this “translator” / renderer does only provide a set of SFC 
functionality, which could mis-lead users over the whole functionality set SFC 
has to offer. 
I think this worth being discuss either at the SFC meeting and/or at the 
Netvirt meeting to clarify things and hopefully come up with some documentation 
around it.
That would be greatly appreciated.

My intuition regarding the fact that the author of the patches is also the one 
who merged them is this was a late release move to get the functionality in 
before the release; bad practice that we unfortunately see a lot when the 
release is getting there.

Thanks,
Alexis

> On Oct 4, 2016, at 3:30 AM, Brady Allen Johnson 
>  wrote:
> 
> 
> I just noticed a merged Gerrit patch in Netvirt [0] to do the Networking SFC 
> to ODL SFC conversion.
> 
> I have to say that I was quite surprised to see that this patch was merged 
> without ever consulting the ODL SFC community. I have a few questions 
> regarding this patch:
> 
> Why is this code in Netvirt and not in SFC where it belongs?
> Why wasnt the SFC community ever informed about these changes?
> Why was the patch +2'd and merged by the author? Unless things have changed 
> drastically lately, self-merging is frowned upon, and in most projects its 
> simply not allowed.
> There are some code review comments on the patch that were never addressed, 
> will these ever be addressed?
> 
> Needless to say, this has left a nasty taste in my mouth. Hopefully we can 
> set this straight and start collaborating between projects in the future.
> 
> Regards,
> 
> Brady
> 
> [0] https://git.opendaylight.org/gerrit/#/c/43305/ 
> 
> 

___
sfc-dev mailing list
sfc-dev@lists.opendaylight.org
https://lists.opendaylight.org/mailman/listinfo/sfc-dev


Re: [sfc-dev] stepping down as a committer

2016-10-04 Thread Brady Allen Johnson

David,

Thanks for all of your help in the past!

I'll take care of this.

Regards,

Brady


On 04/10/16 11:26, Goldberg, David wrote:


Hi

I would like to step down as a committer in the SFC project, since I 
am working on other ODL projects, and I won’t have time to help with SFC.


Thx,

David



___
sfc-dev mailing list
sfc-dev@lists.opendaylight.org
https://lists.opendaylight.org/mailman/listinfo/sfc-dev


___
sfc-dev mailing list
sfc-dev@lists.opendaylight.org
https://lists.opendaylight.org/mailman/listinfo/sfc-dev


[sfc-dev] stepping down as a committer

2016-10-04 Thread Goldberg, David
Hi

I would like to step down as a committer in the SFC project, since I am working 
on other ODL projects, and I won't have time to help with SFC.

Thx,
David
___
sfc-dev mailing list
sfc-dev@lists.opendaylight.org
https://lists.opendaylight.org/mailman/listinfo/sfc-dev


[sfc-dev] Networking SFC convertor in ODL

2016-10-04 Thread Brady Allen Johnson


I just noticed a /*merged*/ Gerrit patch in Netvirt [0] to do the 
Networking SFC to ODL SFC conversion.


I have to say that I was quite surprised to see that this patch was 
merged without ever consulting the ODL SFC community. I have a few 
questions regarding this patch:


1. Why is this code in Netvirt and not in SFC where it belongs?
2. Why wasnt the SFC community ever informed about these changes?
3. Why was the patch +2'd and merged by the author? Unless things have
   changed drastically lately, self-merging is frowned upon, and in
   most projects its simply /*not allowed*/.
4. There are some code review comments on the patch that were never
   addressed, will these ever be addressed?


Needless to say, this has left a nasty taste in my mouth. Hopefully we 
can set this straight and start collaborating between projects in the 
future.


Regards,

Brady

[0] https://git.opendaylight.org/gerrit/#/c/43305/

___
sfc-dev mailing list
sfc-dev@lists.opendaylight.org
https://lists.opendaylight.org/mailman/listinfo/sfc-dev