Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc(XCP-ng): Adding multipathing guideline #316

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ylebris
Copy link
Member

@ylebris ylebris commented Feb 6, 2025

The goal is to add configurations and references guides for iSCSI and Fiber Channel multipathing.

Before submitting the pull request, you must agree with the following statements by checking both boxes with a 'x'.

  • "I accept that my contribution is placed under the CC BY-SA 2.0 license [1]."
  • "My contribution complies with the Developer Certificate of Origin [2]."

[1] https://creativecommons.org/licenses/by-sa/2.0/
[2] https://docs.xcp-ng.org/project/contributing/#developer-certificate-of-origin-dco

The goal is to add configurations and reference guides for iSCSI and Fiber Channel multipathing.

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
@ylebris ylebris requested review from thomas-dkmt and stormi February 6, 2025 14:50
@stormi
Copy link
Member

stormi commented Feb 6, 2025

Has @Fohdeesha reviewed yet?

@ylebris ylebris requested a review from Fohdeesha February 6, 2025 15:01
@ylebris
Copy link
Member Author

ylebris commented Feb 6, 2025

Has @Fohdeesha reviewed yet?

Nothing before tomorrow 😌

Ensure that no spanning-tree is present

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
* iSCSI target ports are operating in portal mode

:::info
It is recommended to configure the network interfaces of XCP-ng host servers, switch interfaces, and storage array interfaces with Jumbo frames (MTU 9000).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All network interfaces, or those dedicated to storage?

I often heard @Fohdeesha say that MTU 9000 is usually not required nowadays, and complain about users who automatically put MTU 9000, not always doing it right, and causing all kinds of networking issues (I can remember a recent nightmarish ticket).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read the entire note.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I had read the entire note. But it doesn't answer my question. So I'll try to put it in a different way: you wrote the network interfaces of XCP-ng host servers, as if they were necessarily all included in a multipath setup. Is that always the case? What about the management interface? The live migration network when there's one? Networks dedicated to VM transfers? Should we be more specific regarding what network interfaces of the host must be configured with jumbo frames?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only talking about iSCSI interfaces, not the others.

This is iSCSI/FC multipathing documentation: all other interfaces are out of scope and will never be mentioned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really really really advise against broadly recommending jumbo frames - on hardware from the last ten or even 15 years, on 10gbE interfaces there is no performance gain - and it opens up a huge box of potential issues that users are going to get themselves into. Something more specific like "if your storage vendor recommends jumbo frames, then do xxx" would be much safer

Copy link
Member

@stormi stormi Feb 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only talking about iSCSI interfaces, not the others.

This is iSCSI/FC multipathing documentation: all other interfaces are out of scope and will never be mentioned.

I thought we had an agreement about this on thursday, after one full hour trying to make you consider my request, and now we're back again to "my document is good as is it, I know my stuff and I won't change it". That's not how peer reviewing works. We have a lot of experience in anticipating how users may interpret stuff the wrong way, and we've spent a lot more time arguing than it would have taken you to add the one or two words that would specify which NICs this applies to.

Your document exists in a broader context, a hypervisor with a complex networking setup, that it's easy to get wrong (ask @Fohdeesha about it). What's obvious to you ("all other interfaces are out of scope and will never be mentioned") is not necessarily obvious to users. When you have the opportunity to make this more obvious, always do. Let's not leave any ambiguity.

Jon's comment also raises important questions, and I kind of agree with him, but I won't take a decision about it for now as it probably requires more internal discussion.

Syntax fix

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
Yann LE BRIS added 2 commits February 6, 2025 18:35
Adding more details

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
Change subtitle to enforce: that it's a new SR

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
Reformulations, modifications, and additions following reviews

Addition of the following sections:
  - Maintenance
  - Troubleshooting
  - Appendix

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
@ylebris
Copy link
Member Author

ylebris commented Feb 10, 2025

@Fohdeesha ready for a new review

…CSI requirements

Simplification of the iSCSI requirements

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
@ylebris ylebris requested review from stormi and Fohdeesha February 10, 2025 17:48
…appendix

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
Copy link
Member

@stormi stormi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good on my side. I leave it to Jon for the final review.

Signed-off-by: Jon Sands <fohdeesha@gmail.com>
Copy link
Member

@Fohdeesha Fohdeesha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also made small spelling fixes in a commit directly to your source branch

## iSCSI

### Requirements
* Four different network interfaces (we recommend using two separate network cards)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would definitely not put four interfaces as a requirement - I can count on two hands how many customers I've seen in four years using this many - the vast majority are using two. Same with multiple network cards, it's nice to have but absolutely not a requirement


### Requirements
* Four different network interfaces (we recommend using two separate network cards)
* Dedicated network interfaces without VLAN tagging on XCP-ng host and storage unit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also not necessary, we have a ton of users and customers running iSCSI storage over vlans, there's nothing wrong with it at all

### Requirements
* Four different network interfaces (we recommend using two separate network cards)
* Dedicated network interfaces without VLAN tagging on XCP-ng host and storage unit
* All network interfaces to the hosts and storage unit **must be set to "STP portEdge"** on the network equipment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly think we should avoid making any kind of network related statements like this, this will totally depend on the customer's environment and isn't related to XCP-ng

* Four different network interfaces (we recommend using two separate network cards)
* Dedicated network interfaces without VLAN tagging on XCP-ng host and storage unit
* All network interfaces to the hosts and storage unit **must be set to "STP portEdge"** on the network equipment
* Two different switches (not stacked) **without Spanning-Tree**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, we really shouldn't make statements like this - lots of customers have STP running on their storage switches just fine to protect against loops. Also stacked switches are fine as well, many, myself included, have ran iSCSI networks over stacked switches

* Dedicated network interfaces without VLAN tagging on XCP-ng host and storage unit
* All network interfaces to the hosts and storage unit **must be set to "STP portEdge"** on the network equipment
* Two different switches (not stacked) **without Spanning-Tree**
* Spanning-tree must be disabled on the switches
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above: not necessarily true, and we need to keep scope in these docs to XCP-ng, not general networking

* All network interfaces to the hosts and storage unit **must be set to "STP portEdge"** on the network equipment
* Two different switches (not stacked) **without Spanning-Tree**
* Spanning-tree must be disabled on the switches
* Two VLANs **without L3 routing**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storage vlans can be routed, it works, it just takes a little more setup. Again I think we should remove these out of scope reqs

The appendix section is out of scope.

Signed-off-by: Yann LE BRIS <yann.lebris@vates.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants