Provide, clean, separated, and commented changes to the upstream project as we
have asked you several times to do.
mpls-linux it is not ready to go upstream in my opinion.
Regards,
Jorge
--
==============================================================
Jorge Boncompte - Ingenieria y Gestion de RED
DTI2 - Desarrollo de la Tecnologia de las Comunicaciones
--------------------------------------------------------------
C/ Abogado Enriquez Barrios, 5 14004 CORDOBA (SPAIN)
Tlf: +34 957 761395 / FAX: +34 957 450380
==============================================================
- There is only so much duct tape you can put on something
before it just becomes a giant ball of duct tape.
==============================================================
Provide, clean, separated, and commented changes to the upstream project as we
have asked you several times to do.
mpls-linux it is not ready to go upstream in my opinion.
Out of tree is out of mind. Very few developers will bother with
a kernel component not in the mainline tree.
Staging?
Provide, clean, separated, and commented changes to the upstream project as we
have asked you several times to do.
mpls-linux it is not ready to go upstream in my opinion.
Out of tree is out of mind. Very few developers will bother with
a kernel component not in the mainline tree.
Yes, I am well aware of it. I've been cleaning the code, as time permits, with
the intention of trying to submit it.
Staging?
I have not really looked at it but I thought that staging is only for drivers?
The mpls-linux code has some (little) hooks in the core networking stuff but I
think that maybe we can remove them and make it almost stand alone.
I would be more than happy to move the code out of the shadows.
--
==============================================================
Jorge Boncompte - Ingenieria y Gestion de RED
DTI2 - Desarrollo de la Tecnologia de las Comunicaciones
--------------------------------------------------------------
C/ Abogado Enriquez Barrios, 5 14004 CORDOBA (SPAIN)
Tlf: +34 957 761395 / FAX: +34 957 450380
==============================================================
- There is only so much duct tape you can put on something
before it just becomes a giant ball of duct tape.
==============================================================
mpls-linux it is not ready to go upstream in my opinion.
And can arguably be implemented in terms of openvswitch and packet scheduler actions.
I guess so, I have not really looked much at the openvswitch code. The MPLS
code it is pretty self contained and in my opinion more tied to routing than to
switching (*). I don't think it should be necessary an userspace daemon, like it
seems necessary for openvswitch, just to bind a label to a route, the same way
that it is not necessary a daemon to support VLAN's currently. The label
signaling protocols are already complicated :)
I work in this code, like a lot of people do, because I have a use for it. I
would really like to see it upstreamed but if it does not fit upstream plans, it
is not a problem, this code it is not that hard to maintain.
In any case critics/reviews/patches are most than welcome.
(*) The code that you implemented for 2.6.1 and that James R. Leu used in this
codebase it is even more tied together to routing.
From: Igor Maravić <hidden> Date: 2011-11-22 08:52:12
What are the main reasons for You to think that mpls-linux is not
ready to go upstream?
As I said, I fixed a lot of bugs so code can be run with all the
kernel hacking options enabled, without causing panics and oopses.
Also fixed a lot of other things and added preprocessor if statements
where code is intertwined with normal kernel code, so I'm sure that it
want break existing kernel compilation if the kernel is compiled
without MPLS.
Why would it be so hard to implemented it in terms of packet scheduler?
BR
Igor
2011/11/21 Jorge Boncompte [DTI2] [off-list ref]:
mpls-linux it is not ready to go upstream in my opinion.
And can arguably be implemented in terms of openvswitch and packet scheduler actions.
I guess so, I have not really looked much at the openvswitch code. The MPLS
code it is pretty self contained and in my opinion more tied to routing than to
switching (*). I don't think it should be necessary an userspace daemon, like it
seems necessary for openvswitch, just to bind a label to a route, the same way
that it is not necessary a daemon to support VLAN's currently. The label
signaling protocols are already complicated :)
I work in this code, like a lot of people do, because I have a use for it. I
would really like to see it upstreamed but if it does not fit upstream plans, it
is not a problem, this code it is not that hard to maintain.
In any case critics/reviews/patches are most than welcome.
(*) The code that you implemented for 2.6.1 and that James R. Leu used in this
codebase it is even more tied together to routing.
What are the main reasons for You to think that mpls-linux is not
ready to go upstream?
As I said, I fixed a lot of bugs so code can be run with all the
kernel hacking options enabled, without causing panics and oopses.
Also fixed a lot of other things and added preprocessor if statements
where code is intertwined with normal kernel code, so I'm sure that it
want break existing kernel compilation if the kernel is compiled
without MPLS.
Why would it be so hard to implemented it in terms of packet scheduler?
First, please, don't top post.
You keep insisting in that you fixed a lot of things, but you have provided a
git tree with just one big commit and say that have taken some of my patches on
it, could you please provide patches on TOP of the sourceforge code for the
things that are not fixed there? And for the new features like the MIB stats
work that you have done? It seems to me that you have not noticed that while
fixing bugs I have reworked a lot of code to make it cleaner or simpler, simply
deleted it and fixed the style.
What's needs to be done, and it's on my TODO list...
The kernel code that is not commented out on the mpls-linux code when you build
the kernel it the shim layer and it's not done on purpose. This code was written
by James to be a generic feature of the networking layer. Now I am not sure that
it has any value keeping it and am for removing it.
The other thing that probably I am going to remove is the labelspace support. I
don't see a use for it, and even Cisco doesn't implement it either that I know.
Then we must rework the netlink interface to make it cleaner and extensible.
Move the tunnel code to use the netlink interface and generic tunnel code.
Check the dst's usages, there has been a lot of changes in the core kernel here
lately and I am not sure if we are using it correctly.
Check the locking and RCU usage.
Look for a way to remove the mpls_ptr from the net_device structure. There's
code on another branch that does a radix-tree lookup for every packet but I
would like something simpler/lighter.
As I said I am not for implementing MPLS support on top of the openvswitch
stuff, that I don't know, like I don't think that we are going to port the
bridge, vlan, ip_gre, or even l2tp support over it, aren't we? :)
I am committed to support this code as best as I can and try to get it merged
but I would be nice if at least I can receive I confirmation of where we are
heading on upstream.
Regards,
Jorge
From: Igor Maravić <hidden> Date: 2011-11-22 13:55:45
2011/11/22 Jorge Boncompte [DTI2] [off-list ref]:
You keep insisting in that you fixed a lot of things, but you have provided a
git tree with just one big commit and say that have taken some of my patches on
it, could you please provide patches on TOP of the sourceforge code for the
things that are not fixed there?
I insist on that because I did do a lot of things. When I did send you
patch, on TOP of your net-next code, about the most important bug
(stack overflow that was happening when mpls nhlfe entry was built)
that I fixed it was just ignored. Unfortunately I started fixing MPLS
code without git, and I added your patches manually.
It seems to me that you have not noticed that while
fixing bugs I have reworked a lot of code to make it cleaner or simpler, simply
deleted it and fixed the style.
Yes I saw that
What's needs to be done, and it's on my TODO list...
The kernel code that is not commented out on the mpls-linux code when you build
the kernel it the shim layer and it's not done on purpose. This code was written
by James to be a generic feature of the networking layer. Now I am not sure that
it has any value keeping it and am for removing it.
I didn't understand what did You want to say here.
The other thing that probably I am going to remove is the labelspace support. I
don't see a use for it, and even Cisco doesn't implement it either that I know.
That's 15 min of work, but I think that labelspaces should stay.
Then we must rework the netlink interface to make it cleaner and extensible.
What do you mean by extensible? With my netlink code you can add,
change and delete ilm, nhlfe and xc entries without any problem. What
other could one wish for? As I could see in your code you can't change
ilm, nhlfe and xc entries.
Check the dst's usages, there has been a lot of changes in the core kernel here
lately and I am not sure if we are using it correctly.
As far I could see you did a great job of using nhlfe as dst entry.
Problem was when you delete nhlfe entry that is referenced with ilm
entry. It shouldn't be allowed to be deleted, or the ilm should change
last instructions from fwd to peek. I did the last thing.
Also there was the problem with neighbor hh_cache, because we are
using nhlfe as dst_entry but I fixed that too. (hh_cache wouldn't
change when we are sending packets with diferent type (ETH_P_IP and
ETH_P_MPLS_UC))
Also ilm shouldn't be dst_entry. I'm going to change that.
Check the locking and RCU usage.
There where few problems about that, that I fixed.
BR
Igor
You keep insisting in that you fixed a lot of things, but you have provided a
git tree with just one big commit and say that have taken some of my patches on
it, could you please provide patches on TOP of the sourceforge code for the
things that are not fixed there?
I insist on that because I did do a lot of things. When I did send you
patch, on TOP of your net-next code, about the most important bug
(stack overflow that was happening when mpls nhlfe entry was built)
that I fixed it was just ignored. Unfortunately I started fixing MPLS
code without git, and I added your patches manually.
(Dropped Dave from the cc: list)
But you failed to provide patches, that fix ONE thing at a time, and not a
patch with all the work that you have done, that's what you sent me for the
stack overflow bug or have done now with the git tree. Providing clean,
separated patches it's YOUR work if you WANT to see them applied on the
mpls-linux tree.
quoted
The kernel code that is not commented out on the mpls-linux code when you build
the kernel it the shim layer and it's not done on purpose. This code was written
by James to be a generic feature of the networking layer. Now I am not sure that
it has any value keeping it and am for removing it.
I didn't understand what did You want to say here.
You have #ifdefed the MPLS code in the core networking code, that's wrong,
that's not the way to go if you want to see the code merged upstream. The shim
layer was thought as a core component of the kernel. If we rip it what we come
at with should be #ifdef-less.
quoted
The other thing that probably I am going to remove is the labelspace support. I
don't see a use for it, and even Cisco doesn't implement it either that I know.
That's 15 min of work, but I think that labelspaces should stay.
Yes, I dit it an hour ago on a private branch. :) Why should did it stay?
quoted
Then we must rework the netlink interface to make it cleaner and extensible.
What do you mean by extensible? With my netlink code you can add,
change and delete ilm, nhlfe and xc entries without any problem. What
other could one wish for? As I could see in your code you can't change
ilm, nhlfe and xc entries.
Yes, I did not finish the change code because no ones uses it currently
(iproute, quagga). In my opinion the instructions should be nested attributes,
and we have to change how the MPLS_CHANGE_* flags get passed, currently it's a hack.
quoted
Check the dst's usages, there has been a lot of changes in the core kernel here
lately and I am not sure if we are using it correctly.
As far I could see you did a great job of using nhlfe as dst entry.
Problem was when you delete nhlfe entry that is referenced with ilm
entry. It shouldn't be allowed to be deleted, or the ilm should change
last instructions from fwd to peek. I did the last thing.
If I delete an nhlfe I for sure don't want the traffic for be routed instead of
dropped. Instructions are policy set by the user and you shouldn't change them
under the hood.
Also there was the problem with neighbor hh_cache, because we are
using nhlfe as dst_entry but I fixed that too. (hh_cache wouldn't
change when we are sending packets with diferent type (ETH_P_IP and
ETH_P_MPLS_UC))
patch.
Also ilm shouldn't be dst_entry. I'm going to change that.
From: Igor Maravić <hidden> Date: 2011-11-22 15:52:04
But you failed to provide patches, that fix ONE thing at a time, and not a
patch with all the work that you have done, that's what you sent me for the
stack overflow bug or have done now with the git tree. Providing clean,
separated patches it's YOUR work if you WANT to see them applied on the
mpls-linux tree.
It will do that when I have free time.
You have #ifdefed the MPLS code in the core networking code, that's wrong,
that's not the way to go if you want to see the code merged upstream. The shim
layer was thought as a core component of the kernel. If we rip it what we come
at with should be #ifdef-less.
If the kernel is compiled witout CONFIG_IP_MPLS we would have intact
core networking code.
That was my idea. Why we would need that code if we don't use MPLS?
quoted
quoted
The other thing that probably I am going to remove is the labelspace support. I
don't see a use for it, and even Cisco doesn't implement it either that I know.
That's 15 min of work, but I think that labelspaces should stay.
Yes, I dit it an hour ago on a private branch. :) Why should did it stay?
Because of RFCs.
Yes, I did not finish the change code because no ones uses it currently
(iproute, quagga). In my opinion the instructions should be nested attributes,
and we have to change how the MPLS_CHANGE_* flags get passed, currently it's a hack.
I like it this way. Don't see the problem with that part of the code.
BR
Igor
From: David Miller <davem@davemloft.net> Date: 2011-11-22 21:49:16
From: Igor Maravić <redacted>
Date: Tue, 22 Nov 2011 22:41:44 +0100
I would like to know what is necesary for MPLS implementation to have,
and to do, so it would be accepted in upstream kernel?
A long and laborious back and forth review process, taking into consideration
not just the technical details of the patches themselves, but the top level
and overall design.
That's what it will take.
Taking someone else's work, fixing all the bugs and cleaning them up is
far from sufficient for a feature of this nature. There is natural
overlap all over and we have to make sure the implementation bits are
going into the right places.
One issue of constant contention is that people want to add all of their
favorite packet filtering and packet mangling into their protocol handling
code, with all kinds of custom controls and configuration mechanisms.
WE HATE THIS.
We have the packet scheduler classifiers and packet actions for a reason,
and we want them to used instead of ignored.
We are going through the same thing in the review process for the openvswitch
code, which brings up another design question for MPLS, which is whether MPLS
can be better implemented in terms of openvswitch.
You're in the unfortunate position of submitting a feature that has a
lot of overlap with many other subsystems, existing code, and features
being submitted at the same time. We want as much reuse as possible,
and we want it all designed right before it gets integrated.
I frankly don't care very much about MPLS personally, it's such a
fringe facility. So if people just argue themselves into oblivion and
no forward progress is made, just like last time an MPLS submission
was attempted, that's also fine with me :-)
From: Igor Maravić <hidden> Date: 2011-11-23 07:09:36
OK,
Thanks
2011/11/22 David Miller [off-list ref]:
From: Igor Maravić <redacted>
Date: Tue, 22 Nov 2011 22:41:44 +0100
quoted
I would like to know what is necesary for MPLS implementation to have,
and to do, so it would be accepted in upstream kernel?
A long and laborious back and forth review process, taking into consideration
not just the technical details of the patches themselves, but the top level
and overall design.
That's what it will take.
Taking someone else's work, fixing all the bugs and cleaning them up is
far from sufficient for a feature of this nature. There is natural
overlap all over and we have to make sure the implementation bits are
going into the right places.
One issue of constant contention is that people want to add all of their
favorite packet filtering and packet mangling into their protocol handling
code, with all kinds of custom controls and configuration mechanisms.
WE HATE THIS.
We have the packet scheduler classifiers and packet actions for a reason,
and we want them to used instead of ignored.
We are going through the same thing in the review process for the openvswitch
code, which brings up another design question for MPLS, which is whether MPLS
can be better implemented in terms of openvswitch.
You're in the unfortunate position of submitting a feature that has a
lot of overlap with many other subsystems, existing code, and features
being submitted at the same time. We want as much reuse as possible,
and we want it all designed right before it gets integrated.
I frankly don't care very much about MPLS personally, it's such a
fringe facility. So if people just argue themselves into oblivion and
no forward progress is made, just like last time an MPLS submission
was attempted, that's also fine with me :-)
On Tue, 2011-11-22 at 16:49 -0500, David Miller wrote:
I frankly don't care very much about MPLS personally, it's such a
fringe facility. So if people just argue themselves into oblivion and
no forward progress is made, just like last time an MPLS submission
was attempted, that's also fine with me :-)
Hello David,
Oh dear. I don't know how I can express my years of frustration at the
lack of MPLS in the stock Linux kernel and the thought that it may be
another five years away.
Maybe that I've just spend $50K on a router to terminate MPLS tunnels
into ethernet VLANs, just so Linux can be a recording device for Juniper
routers intercepting traffic. You might well ask why Juniper chose MPLS
rather than GRE, and the answer there is that MPLS is so fundamental to
modern networking that it is implemented in the forwarding silicon of
the interface, making MPLS the obvious choice for copying every packet
matching a flowspec into a tunnel
Maybe that MPLS is the technology used to transform a router into a
network. Which is why all of the Linux router and software-defined
networking devices have kernels with MPLS patches. Middleboxes lacking
MPLS are increasingly difficult to integrate into a modern ISP network
-- firewalls, SIP session border controllers, etc. Linux is, of course,
the dominant OS on those middleboxes.
Maybe that without host MPLS we are only left with the architectural
mess which is data centre ethernet when wanting to add advanced
networking features to hosts. There's a good argument that data centre
ethernet only exists because MPLS isn't widespread on hosts.
Maybe that servers hosting virtual machines forces servers to become
part of the network -- servers aren't edge devices anymore. Terminating
MPLS on an edge subnet is difficult when the edge subnet exists within a
server which doesn't implement MPLS.
Maybe that the server is changing so that advanced networking is part of
its brief. Large sites have redundant data centres. Small sites
outsource to cloud providers to gain the advantages of large sites. The
pool outside of those two is shrinking.
I don't know enough to say if these MPLS patches are any good or not --
I haven't spent my life working on the Linux kernel. I have spent my
life building Internet networks, so I do know enough to say that if you
want Linux to continue to be attractive to ISPs and large enterprises as
the Swiss Army Knife for network services then it is well time that some
MPLS implementation was in the stock kernel.
Otherwise the Linux networking implementation will simply become
irrelevant. People with deep networking needs -- ISPs, enterprise data
centres, large content sites -- will simply use Linux to implement the
interfaces and attach them to a VM running more capable networking
software from C or J. You're already seeing software from these people
now, because the thought of them getting revenue for existing software
with no hardware development makes them drool.
I've long admired the quality of the Linux networking implementation.
For example, router manufacturers could learn a lot from the deep
thought and clear design of the "tc" subsystem. The work which was done
to make TCP perform well is outstanding. I very much want Linux
networking to continue to succeed. So please take this suggestion that
it is well time for forward progress on MPLS in the spirit it is meant.
My apologies where you may feel I have vented excessively above.
Best wishes, Glen
--
Glen Turner <http://www.gdt.id.au/~gdt/>
On Tue, 2011-11-22 at 16:49 -0500, David Miller wrote:
quoted
I frankly don't care very much about MPLS personally, it's such a
fringe facility. So if people just argue themselves into oblivion and
no forward progress is made, just like last time an MPLS submission
was attempted, that's also fine with me :-)
Hello David,
Just wanted to let you know that with the workload of patch review and
coding I have, I basically have zero time for verbose editorials like
this and, without exception, I never read them.