Runtime: PhysicalAddress.Parse should support more formats

[iSazonov]

Motivation

From https://www.lifewire.com/introduction-to-mac-addresses-817937 currently follow formats and separators is used by different vendors:

MMMMMMSSSSSS
MM:MM:MM:SS:SS:SS
MM-MM-MM-SS-SS-SS
MMM.MMM.SSS.SSS

We need an API to support this.

1. We could have method like mac.ToString(":"); with custom separator/format provider to get MACs in all common and custom formats.
2. Many APIs add TryParse() in addition to Parse() for hot code paths to avoid exceptions that could be important for networking scenarios.

(Related dotnet/corefx#41518)

Proposal

```c#
namespace System.Net.NetworkInformation
{
public class PhysicalAddress
{
public static bool TryParse(string address, out PhysicalAddress mac);

    //public string ToString(); -- Existing API
    public string ToString(string? format);
    public string ToString(string? format, IFormatProvider? provider);
}

}

Original issue for history:
--------------

[System.Net.NetworkInformation.PhysicalAddress.Parse()](http://source.dot.net/#System.Net.NetworkInformation/System/Net/NetworkInformation/PhysicalAddress.cs,118) method [currently support](https://msdn.microsoft.com/en-us/library/system.net.networkinformation.physicaladdress.parse%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396) only following formats:

001122334455

00-11-22-33-44-55

Really we can find that [more formats](https://www.lifewire.com/introduction-to-mac-addresses-817937) is used by verdors and humans:

00:11:22:33:44:55
0011:2233:4455

So we could have extention method `mac.ToString(":");` with custom separator and we should parse mac address with ':' or custom separator.

Many APIs add TryParse() in addition to Parse().

Also docs says that the Parse() method only support upper-case letters as hexadecimal digits but some application and humans can type macs in lower-case - it will be not so big overhead to make the method _case-insensitive_.

```c#
namespace System.Net.NetworkInformation
{
    public class PhysicalAddress
    {
        public static bool TryParse(string address, out PhysicalAddress mac);

        public static PhysicalAddress Parse(string address); // make case-insensitive

        public string ToString();
        public string ToString(string? format);
        public string ToString(string? format, IFormatProvider? provider);
    }
}

[karelz]

How often humans type MAC addresses? I haven't have to use it so far.
Is the scenario copy-paste from some tooling for scripting scenarios?

Case-insensitive Parse sounds reasonable. So does TryParse variant of parsing. Allowing : or . as separator, is also reasonable.
Question: I didn't see the form 0011:2233:4455 in the linked article, where did it come from.
Open question: Should we parse . only in form of MMM.MMM.SSS.SSS or also MM.MM.MM.SS.SS.SS?

The ToString(char) makes sense as reverse operation to Parse, however, it is not clear how it should behave for . separator - should it follow 3-digits or 2-digits separation format?

BTW: The code is here:
https://github.com/dotnet/corefx/blob/bffef76f6af208e2042a2f27bc081ee908bb390b/src/System.Net.NetworkInformation/src/System/Net/NetworkInformation/PhysicalAddress.cs#L118

[iSazonov]

How often humans type MAC addresses? I haven't have to use it so far.

I don't know "how often" but I know some scenarios. Every network hardware has a sticker with printed MAC. If I have to type the MAC I really type it (in lower-case). Network printers can print configurations. Then I receive the paper and type MAC. Sometimes users report the address of their equipment by phone.
If we talk about apps then it may be Excel, sniffer, PowerShell script, web app.

Question: I didn't see the form 0011:2233:4455 in the linked article, where did it come from.
Open question: Should we parse . only in form of MMM.MMM.SSS.SSS or also MM.MM.MM.SS.SS.SS?

There seems to be no standard for this separator. The most popular is the dash and colon.
_In fact, we could use an arbitrary separator in any position, provided that it is the same in the entire address string_.

[karelz]

Is there any specific scenario driving the changes now? Any real application which hit the limitations?

In fact, we could use an arbitrary separator in any position, provided that it is the same in the entire address string.

I don't think it is a good idea. It is way too free-form IMO and goes against the validation purpose. If some scenarios need it, they should do custom parsing.

There seems to be no standard for this separator. The most popular is the dash and colon.

Then I would go with what is most commonly used - I assume the linked article is a good start, let's go after the formats listed there. If you know about other articles/books that list more typical formats, we can consider adding them as well.

[iSazonov]

Is there any specific scenario driving the changes now? Any real application which hit the limitations?

I can't speak for everyone. Today I looked through the PowerShell scripts that handle MAC addresses and found the skipped APIs. They use Regex to replace separators and String.ToUpper(). Using the latter obviously forces C# developers to do extra allocations.
We could add ReadOnlySpan overload to the APIs:
```c#
namespace System.Net.NetworkInformation
{
public class PhysicalAddress
{
public static bool TryParse(string address, out PhysicalAddress mac);
public static bool TryParse(ReadOnlySpan address, out PhysicalAddress mac);

    public static PhysicalAddress Parse(string address); // make case-insensitive
    public static PhysicalAddress Parse(ReadOnlySpan<char> address);

    public string ToString(string separator);
}

}
```

I would go with what is most commonly used

I agree.

[karelz]

We should add Span overloads only where perf matters, otherwise it would be everywhere, not always for good reason.
I don't see the scenario above as perf-critical. Moreover the parsing is not rocket science and anyone can write their own methods if they care deeply enough (e.g. over Span).

[iSazonov]

We should add Span overloads only where perf matters,

Is there already a best practice guide for Span? It would be very useful to have such a guidance.

[iSazonov]

Let's to consider a scenario of DHCP log parsing. We read line by line and parse the line to get fields. Previously we used Substring() method. My understanding is that now best practice (from Stephen Toub) is to use Slice() to exclude extra allocations. So suggested overloads with ReadOnlySpan can be useful.
c# ReadOnlySpan<char> line = ... ReadOnlySpan<char> macStr = GetMacField(line); PhysicalAddress mac = PhysicalAddress.TryParse(macStr);

[karelz]

@iSazonov yes, I can imagine such scenario. Is MAC address parsing truly perf-sensitive part of the scenario? Is it common scenario for apps and libraries?
The problem is that anyone can come up with some (potentially perf-sensitive) scenario for almost any API in BCL. We add just the most important ones, where real apps and libraries benefit most. This API seems to be fairly rare (0.1% in API usage).
If there was a real application/library where it would show measurable benefit, I'd be more inclined to add it.

@stephentoub any comments / guidance on adding Span overloads to BCL APIs?

[iSazonov]

Yes, it is very perf-sensitive - sizes of DHCP logs can reach _huge_ sizes in enterprises. We could use perl or php (or sed/awk) but why we should refuse favourite C# and PowerShell?

Yes, the API is rare. (1) Although it is rare but we all already have this Parse (string address); - why if it is rare? (2) Why such APIs should remain not optimized? Adding and having ReadOnlySpan overloads is too expensive? I'd like to see a policy and guidance from your team.

[ghost]

I think there are different things been discussed.

Initially:

• Change the behavior of MAC address parser to support:
  
  
  
  • all known MAC formats (not just IEEE 802?)
  
  
  • make the parser insensitive
  
  
• Add TryParse method - API proposal

Later:

• Add ReadOnlySpan overloads for Parse and TryParse - API proposal

I'd open separate issues for parser fixes and API additions and keep conversation scoped to the issue at hand.

[danmosemsft]

Yes please let's use this issue for casing/separators and please open a single new issue for the API suggestions on the format suggested in https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md

[iSazonov]

New issue for ReadOnlySpan overloads dotnet/corefx#29780

[karelz]

Triage:

• We are ok with case-insensivity (that is just a bug fix), also with parsing 2 new formats above (also a bug fix/enhancement) -- we do not need API review for these.
• We are OK with TryParse addition -- please create separate API proposal and let's get it through API review or let's reuse this one.
• We recommend to change ToString from character to format string (same way as GUID). Separator is just too restrictive long-term.

[iSazonov]

@karelz I believe we could move the case-insensivity (that is just a bug fix) to new issue and leave TryParse and ToString here for follow API review.

ToString proposal was updated to format string like GUID.

[karelz]

@iSazonov sounds good. Can you please clean up top post and remove the not-relevant-anymore parts of it? So that whoever comes can see latest info in top post.
Please also clarify which APIs are new vs. existing (ToString()?).

[iSazonov]

@karelz Done! Please edit top post if I skipped something.

[karelz]

@iSazonov thanks! I removed Parse as discussed above. I marked existing API clearly.
I don't quite follow [1] there. Can you please clarify it and spec it out here? We need specific proposal of behavior / format.

[iSazonov]

@karelz I don't understand you request about [1] - I opened dotnet/corefx#41518 for this.

[karelz]

@iSazonov we need full description of the proposed format in [1] ToString(":") -- see our triage comments in https://github.com/dotnet/corefx/issues/29740#issuecomment-537243912
This part is not clear: "_with custom separator and we should parse mac address with ':' or custom separator_"

It would be best to provide specific examples of the API usage to avoid any confusion. See API process for motivation and examples.

[iSazonov]

Will do.

[danmosemsft]

Hi @iSazonov did you get a chance to provide info requested?

[iSazonov]

@danmosemsft All info about formats was explicitly added.

MM:MM:MM:SS:SS:SS
MM-MM-MM-SS-SS-SS
MMM.MMM.SSS.SSS